Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add regression build #6577

Closed
ilyavolodin opened this issue Jul 2, 2016 · 18 comments
Closed

Add regression build #6577

ilyavolodin opened this issue Jul 2, 2016 · 18 comments
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion build This change relates to ESLint's build process feature This change adds a new feature to ESLint

Comments

@ilyavolodin
Copy link
Member

Given that bugs slip through code reviews from time to time, I think it would be worth creating regression build that we would run before the release. The idea would be to find a few large repositories that are already using ESLint (hopefully with variety of rules, plugins and parsers), have them automatically cloned, reset to a specified tag, and linted using version that is about to be released. Then output should be verified against baseline.
This have potential for a bunch of false positives and will most likely require manual verification, however, it can be used as early warning system if the output would allow to easily compare changes between baseline output and new.
We could even try to set it up to run once a day, and open an issue if there are major differences with output. (although I don't think Travis allows for scheduled builds).

@ilyavolodin ilyavolodin added build This change relates to ESLint's build process feature This change adds a new feature to ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Jul 2, 2016
@nzakas
Copy link
Member

nzakas commented Jul 4, 2016

Duplicate of #4500?

@ilyavolodin
Copy link
Member Author

Partially. That one is mostly about plugins, this one is overall regression build that should include our own rules, plugins and parsers. I can close this one in favor of #4500 though.

@nzakas
Copy link
Member

nzakas commented Jul 4, 2016

What bugs do you envision this catching?

@ilyavolodin
Copy link
Member Author

Bugs like #6576 for example.

@nzakas
Copy link
Member

nzakas commented Jul 5, 2016

Hmmm, I'm not sure that would have been caught, but I do think it's a good idea anyway.

Only question is how would we do this? And when would this test be run? (In some cases a break will be expected, so not all failures are bad)

@ilyavolodin
Copy link
Member Author

Correct. As I noted in the original post, regression build would require some manual verification, since there will be false positives. Travis supports cron jobs we could implement regression build as such. I would say that we can start with running it every week on the Thursday night, this way we would have a result of regression build before doing a release on Friday. We can tweak the schedule as we see fit once we understand how many false positives there are.

@nzakas
Copy link
Member

nzakas commented Jul 8, 2016

From what I know, the cron jobs just run your normal Travis build on a schedule, you can't have a different build.

I've got a new VM that I'll setup Jenkins on. That's probably going to be easier.

@ilyavolodin
Copy link
Member Author

Sounds good to me. Let me know if you need some help with this. Should this issue be accepted.

@mgol
Copy link
Contributor

mgol commented Jul 8, 2016

Hmm, the Travis docs for the cron jobs mention an environment variable specifying you're in the cron job available:
https://docs.travis-ci.com/user/cron-jobs/#Detecting-Builds-Triggered-by-Cron
Is it not enough to do sth like:

if (process.env.TRAVIS_EVENT_TYPE === 'cron') {
    // Do the regression tests
} else {
    // Run normal tests
}

?

It's true you can do more on Jenkins but Jenkins has to be managed, updated etc. which is not always easy to keep up with so if sth can be done with Travis it may be easier in the long run. Or am I not seeing sth?

@nzakas nzakas added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Jul 8, 2016
@nzakas
Copy link
Member

nzakas commented Jul 8, 2016

@mgol ah, that's interesting. I always feel like branching conditionals in this way is a great way to cause trouble without realizing it. IMHO, it's best to have something completely separate so as to avoid trouble.

@ilyavolodin
Copy link
Member Author

I'm trying to setup regression build, but I'm having very hard time finding projects that I can use. Any suggestions would be appreciated. Here's what I've tried:

  • NodeJS - Their setup is really strange. I'm not even sure how they run lint. ESLint seems to be cloned into their repository.
  • Underscore - They are using ESLint version 1.10, so using latest outputs a bunch of errors
  • React - I can't even NPM install it on Jenkins server. They are using ESLint through Grunt, that spawns Gulp process (I know). And babel-eslint doesn't want to run with latest version.
  • Karma - Can't NPM install because of PhantomJS.

I'm looking for large projects using AirBnB, Standard and XO. If you know any without major dependencies, let me know please.

@nzakas
Copy link
Member

nzakas commented Jul 12, 2016

How about jQuery and Babel?

@ilyavolodin
Copy link
Member Author

Haven't tried jQuery, I'll try it. Although they are not using any plugins/parsers, so it will only cover core rules. I have tried Babel, but I don't remember what were the problems I ran into.

@ilyavolodin
Copy link
Member Author

I have a passing build that does the following:
eslint-plugin-react
eslint-plugin-html
eslint-plugin-import

Clone, upgrade ESLint and run unit tests

jquery

Clone, upgrade ESLint and run lint task (through Grunt)

I can either enable it now and keep adding more as we run it, or I can hold on, and add more before enabling it.

@nzakas
Copy link
Member

nzakas commented Jul 15, 2016

If there's no downside to enabling, I'd say turn it on now.

@hzoo
Copy link
Member

hzoo commented Jul 15, 2016

Babel uses babel-eslint; I should be able to help with issues there

@ilyavolodin
Copy link
Member Author

@hzoo That would be great. I can't even figure out how exactly does babel run eslint:-) If you could provide a small bash script that would clone babel, update eslint and run linting, that would be great. Here's a quick example for jQuery:

git clone https://github.com/jquery/jquery.git
cd jquery
npm install
node node_modules/grunt-eslint/node_modules/eslint/bin/eslint -v
cd node_modules/grunt-eslint
npm install git+https://github.com/eslint/eslint.git
cd ../..
node node_modules/grunt-eslint/node_modules/eslint/bin/eslint -v
npm run precommit
cd ..

@ilyavolodin
Copy link
Member Author

Regression build is setup and running. It will send an email to eslint-team google group whenever there's a broken build. In the future we should add more projects to it, as well as automate it to open up a new issue in this repository on the broken build, instead of sending an email.
Closing for now, since we are doing some regression testing now.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion build This change relates to ESLint's build process feature This change adds a new feature to ESLint
Projects
None yet
Development

No branches or pull requests

4 participants