Fail fast strategy for JSHint/JSCS/ESLint tests

[simonihmig]

Just had the following happen: pushed a commit that accidentally had two consecutive blank lines in a file, which is a violation of a JSCS rule (ember-suave). The build took quite some time, just to show that a single JSCS test was failing for all ember versions (1.13 - 2.8 + beta + canary). Not only does this hamper developer productivity, it also puts a considerable amount of unnecessary burden on the CI infrastructure (Travis in this case).

Since JSHint/JSCS/ESLint tests won't depend on any dependencies, a) it seems unnecessary to run them in every try:each run and b) it could make sense to have them run once before any "real" tests.

So a "fail fast, fail early" strategy, which I guess is a pretty common CI best practice, could be in this case:

1. run all JSHint/JSCS/ESLint tests with the default deps
2. If that fails -> exit (non zero code)
3. run try:each, with JSHint/JSCS/ESLint tests excluded

I guess you could make 1+3 happen with a custom config, not sure about 2? And what do you think about this approach, maybe this could become the default?

[kategengler]

This is a bit outside the scope of ember-try.

ember-try runs a command (by default ember test) with each scenario of dependencies specified. It technically doesn't know anything about the command that is being run, other than whether or not it exits non-zero.

You can accomplish not running those linters with ember try:each this by using a custom test_page URL in testem.js or as a command line argument to the ember test command (or the command specified in your ember-try.js config).

As for failing fast, that would be up to whatever command is being run. With a short search I didn't find any way for either QUnit or testem to fail fast, though.

[simonihmig]

Sure, it should not be built-in into ember-try, that was not what I was trying to suggest. But it would be cool if this could be achieved through a custom config.

As I said in my first comment, it seems step 2 is currently not possible, i.e. to skip all following scenarios if one previous has failed. Let's say there is a new flag failFast, that says that all following scenarios should be skipped if this one fails. A config could look like this:

{
  scenarios: [
    {
      name: 'jshint',
      command: 'ember test -f jshint',
      failFast: true
    },
    {
      name: 'default',
      bower: {
        dependencies: { }
      }
    },
    {
      name: 'ember-release',
      bower: {
        dependencies: {
          ember: 'release'
        }
      }
    },
    ...
  ]
}

If the jshint scenarios fails, all following scenarios would be skipped, so ember-try would exit immediately with a non-zero exit code.

This would assume a semantic that all scenarios are always processed sequentially and in order. If that would not be the case in the future (i.e. parallel processing), you could say that no new scenario should be started after a failFast scenario has failed.

Does that make sense?

[rwjblue]

Does that make sense?

Sure, that does seem neat.

[kategengler]

Ah, I thought you wanted the test command to fail as soon as it encountered a failure, which would be up to QUnit/Testem.

I'll have to admit I'm still confused by the general desire to have ember-try scenarios where no dependencies are changed. To me, the entire point of ember-try is the changing of dependencies.

Today, you can get this behavior by doing:

package.json

"scripts": {
    "build": "ember build",
    "start": "ember server",
    "test": "ember test -f jshint && ember test -f jscs && ember try:each"
  },

ember-try.js

   command: "ember test --query 'nolint&nojscs'"
   scenarios: {}  // not necessary just showing that 'command' is outside of scenarios

With this setup, you can run npm test and it won't run any of the linters fails.

That said, I do think an option to fail after the first failed scenario would be a nice feature for ember-try. It won't help on CI for anybody using the Travis matrix (I couldn't find an option to fail after the first failed member of the build matrix), but should help in dev and for anybody using ember try:each in CI.

[rwjblue]

I think this is still a good idea. 😸