Abort old tests and start new ones after a code change in watch mode

[vadimdemedes]

Follow up from #502 (comment).

[novemberborn]

Would have to restart the old tests as well as any new ones.

Are there any concerns around cleanup code not running when tests are aborted?

[jamestalmage]

Are there any concerns around cleanup code not running when tests are aborted?

I would say yes. We could make it configurable. That would mean watch support now adds three cli flags - which seems a lot.

Or - the keyboard command could trigger the immediate rerun. I like this one I think. I only care about this on projects with really long running test suites.

[sindresorhus]

Cleanup code should be in the after/afterEach hooks, so why not run them when tests are cancelled?

[vadimdemedes]

Cleanup code should be in the after/afterEach hooks, so why not run them when tests are cancelled?

Exactly what should be done. I imagine sending "abort" event to forked process, which would prevent execution of next steps and run the after hooks.

[jamestalmage]

If you have all concurrent tests then this behavior is essentially useless. Tests will have begun by the time you send an abort. So if we follow up with afterEach then after its really no different than a standard test run.

[novemberborn]

Or - the keyboard command could trigger the immediate rerun. I like this one I think. I only care about this on projects with really long running test suites.

At least that's more or less equivalent to sending a SIGINT and restarting manually.

[vadimdemedes]

@jamestalmage I think you misunderstood my idea.

Say we have long-running tests, does not matter how much. Inagine each of them takes at least a second. When in watch mode, after I make a change, I don't need to see the test results from the old test run, I want to see new tests immediately starting. So we need to abort & clean up the previous tests, in order to start new ones asap.

[jamestalmage]

I think I know what you are asking, and I am not objecting to the concept in general, I am just saying I don't see a practical way of accomplishing it as currently proposed.

How are you planning on "aborting"? What do you do when you receive the "abort" message from the parent process? You can't interrupt tests that are already launched. You could elect not to execute the next hook, but so far everyone seems to be saying they want after/afterEach to run. You could avoid executing the next serial test, but then this is only really useful for people using a large number of serial tests (which, if we do a good job explaining the core concepts of AVA, should be small subset of our user base).

IMO the easiest way to get at your basic request ("I want to see new tests immediately starting") is to just kill the child processes. This is problematic if there is cleanup code in after / afterEach that means test fixture state is left uncleaned. Having your tests rely on system state is a terrible idea anyways, so I would be fine with just killing child process - but the consensus so for seems to be that we should still run those post test hooks.

That's how I came to the conclusion that we should allow either:

• A config option that tells AVA your after, afterEach are not needed for cleanup, and that processes can be killed immediately to run the next watch cycle.

or

• to avoid adding yet another config option, allow the keyboard shortcut to force that condition

[vadimdemedes]

What I meant in terms of implementation is much simpler and does not need to kill the process, add one more config option and other mess. Simply notify forked process, that no more tests should be run. Let the currently running tests and their hooks finish to properly clean up everything, but don't run the next concurrent/serial test.

[jamestalmage]

There is no such thing as a "next concurrent test". Every concurrent test in the file is launched in a single turn of the event loop, once you have kicked off the "run the concurrent tests" phase, there is no turning back. Any ipc message received during initiation of concurrent tests will block until all concurrent tests have been started.

Things are a bit different if they've got a lot of serial tests (especially async ones). In that case there is an opportunity to interrupt execution flow. I am just not sure it's worth the effort for what should be a niche situation (people should prefer concurrent). That said, large groups of async serial tests are likely to be the slowest part of your test suite, so maybe. I think we should just wait until someone presents a test suite where this would be of significant benefit.

[vadimdemedes]

James, you are right about concurrent tests, can't believe I actually suggested that.

As for serial tests, example of such test suite is Ghost blogging software, almost all their tests rely on database state, so they have to clean it before each new test. Their test suite takes 11m to run. If we wouldn't abort old tests on new code update, watch mode becomes useless in that case.

[sindresorhus]

Every concurrent test in the file is launched in a single turn of the event loop, once you have kicked off the "run the concurrent tests" phase, there is no turning back.

What I was thinking is that even though we have started the concurrent tests we can still choose to ignore their result and pretty much unref / mark them as aborted, and ignore. Like an unref'd/detached child process, it still runs to completion, but the parent process don't care and have no communication with it anymore.

[vadimdemedes]

@sindresorhus yes, I think this is the best we can do in that situation, great idea!

[novemberborn]

What I was thinking is that even though we have started the concurrent tests we can still choose to ignore their result and pretty much unref / mark them as aborted, and ignore. Like an unref'd/detached child process, it still runs to completion, but the parent process don't care and have no communication with it anymore.

That would work as long as there are no implicit assumptions that test files are not executed concurrently. If a test file resets a static database table whilst cleaning up that may break a concurrent run of that same test file.

[sindresorhus]

@novemberborn Yes, but the idea is that the after/afterEach will run right away when it's unref'd.

[novemberborn]

but the idea is that the after/afterEach will run right away when it's unref'd.

While the tests themselves are still running? That'd be fine for reads (they'll fail but their output is discarded) but not for writes, which will no longer be cleaned up.

[novemberborn]

With #78 in place we should make sure any scheduled test files are aborted. #710 will give us the ability to abort test runs. This is blocked until those issues are resolved.

[novemberborn]

Our --fail-fast implementation can pre-empt tests that haven't yet started. That functionality could be used to make sure a test process exits more quickly before rerunning it.