Skip to content

Refactor how test files are told to run their tests, and more #1693

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

Merged
merged 17 commits into from
Feb 11, 2018

Conversation

novemberborn
Copy link
Member

@novemberborn novemberborn commented Feb 6, 2018

Apologies, this is a big one! See the commits for details, but the highlights are:

  • Remove cruft remaining from when we switched to always using the limited concurrency mode.
  • The cache directory is now always relative to the project directory. Previously another unnecessary path lookup was done, which I think may have resulted in different cache paths if there were package.json files at different levels of the project.
  • Compile all test and helpers files before running any test. This doesn't improve time-to-first-test but it does mean that test files can depend on each other. AVA will correctly load the precompiled file. No tests are run if any file fails to compile.
  • The real file path is passed to the precompiler.
  • execArgv is computed when running a test file, not all at once for all files.
  • --fail-fast now works with limited concurrency mode, preventing new test files from being run. It also fails fast after a timeout or if a test file crashed. This is part of the work needed for --fail-fast keeps running other test files #1158.
  • The mini and verbose reporters print the number of untested files after --fail-fast.
  • Improved output for unhandled rejections / uncaught exceptions in the mini and verbose reporters.
  • Test runners will now run tests as soon as (in the next tick) they're declared. The main process no longer instructs the workers to startrunning tests. Fixes Collecting and running tests needs some cleanup #1674.

Remove indirection by inlining code. It's not the prettiest without
async-await but it's easier to follow.

Remove unnecessary auto-binding of methods. There's no need to "get"
blank results either when running a test files fails.

Handle timeouts within a test run, without touching the `runStatus`
object. Restart the timeout timer whenever a new "fork" is created. This
gives new test files the chance to start running tests. Don't start the
timer *before* running any tests. Fixes #1377.

Clean up "fork" instances when test files exit.

Set up a new precompiler on every test run. Don't store it on the `Api`
instance. If caching is enabled, fix the caching directory relative to
the project directory, without looking it up again.

Compile all test and helpers files before running any test. This doesn't
improve time-to-first-test but it does mean that test files can depend
on each other. AVA will correctly load the precompiled file. No tests
are run if any file fails to compile.

Pass the real path to the precompiler.

Compute `execArgv` when a test file is run, not all at once for all
files.
Don't run new test files after:

* A timeout has occurred
* A test has failed
* Running the test file itself caused an error

Refs #1158.
This module is necessary since there may be circular dependencies
originating in test-worker.js. Make setting and getting the options
explicit, prevent options from being replaced and require them to be
set. This will help catch any future issues caused by changing the
dependency tree.
Replace lib/globals.js by a module which focuses on reexporting Node.js'
timers and Date.now().

Remove workarounds from profile.js, presuming they're outdated and don't
apply to methods from Node.js' `timers` module.
…ters

* Make error type more readable
* Always include originating test file
* Indent error summary, allowing for multi-line summaries
* Correctly handle non-errors in verbose reporter
SyntaxError stacks may begin with the offending code. Include all stack
lines up to and including one that begins with SyntaxError.
This focuses lib/main.js on creating and exporting the runner, while the
worker focuses on managing IPC, catching errors, and doing generally
doing the work.

The worker now guards against repeatedly sending stats, or sending test
results after it's started to exit.

Required and test files are loaded at the end, after handlers for
uncaught exceptions and unhandled rejections have been installed.
Test runners will now run tests as soon as (in the next tick) they're
declared. The main process no longer instructs the workers to start
running tests.

Various IPC commands had to be changed to ensure we can still detect
whether "ava" is loaded, or whether a test file contains no actual
tests.

Fixes #1674.
Ensure the IPC channel stays referenced. This is a safe-guard: it's easy
to end up in a situation where the channel needs to be referenced but
it's unreferenced because other code wasn't aware.
There are no more anonymous tests.
@novemberborn
Copy link
Member Author

Additionally:

Fixes #1684. Fixes #1416. Refs #1158.

Properly support serial hooks. Hooks are divided into the following categories:

  • before
  • beforeEach
  • afterEach
  • afterEach.always
  • after
  • after.always

For each category all hooks are run in the order they're declared in. This is different from tests, where serial tests are run before concurrent ones.

By default hooks run concurrently. However a serial hook is not run before all preceding concurrent hooks have completed. Serial hooks are never run concurrently.

Always hooks are now always run, even if --fail-fast is enabled.

Internally, TestCollection, Sequence and Concurrent have been removed. This has led to a major refactor of Runner, and some smaller breaking changes and bug fixes:

  • Unnecessary validations have been removed
  • Macros can be used with hooks
  • A macro is recognized even if no additional arguments are given, so it can modify the test (or hook) title
  • --match now also matches todo tests
  • Skipped and todo tests are shown first in the output
  • --fail-fast prevents subsequent tests from running as long as the failure occurs in a serial test

Fixes #1684. Fixes #1416. Refs #1158.

Properly support serial hooks. Hooks are divided into the following
categories:

* before
* beforeEach
* afterEach
* afterEach.always
* after
* after.always

For each category all hooks are run in the order they're declared in.
This is different from tests, where serial tests are run before
concurrent ones.

By default hooks run concurrently. However a serial hook is not run
before all preceding concurrent hooks have completed. Serial hooks are
never run concurrently.

Always hooks are now always run, even if --fail-fast is enabled.

Internally, TestCollection, Sequence and Concurrent have been removed.
This has led to a major refactor of Runner, and some smaller breaking
changes and bug fixes:

* Unnecessary validations have been removed
* Macros can be used with hooks
* A macro is recognized even if no additional arguments are given, so it
can modify the test (or hook) title
* --match now also matches todo tests
* Skipped and todo tests are shown first in the output
* --fail-fast prevents subsequent tests from running as long as the
failure occurs in a serial test
If a failure occurs in one worker, attempt to interrupt
other workers. This only works as long as the other worker has not yet
started running concurrent tests.

Fixes #1158.
@novemberborn novemberborn changed the title Refactor how test files are told to run their tests, and fallout Refactor how test files are told to run their tests, and more Feb 11, 2018
@novemberborn
Copy link
Member Author

And finally, if a failure occurs in one worker, attempt to interrupt other workers. This only works as long as the other worker has not yet started running concurrent tests. Fixes #1158.

@novemberborn novemberborn merged commit c1b418f into master Feb 11, 2018
@novemberborn novemberborn deleted the refactor-run branch February 11, 2018 16:12
@sindresorhus
Copy link
Member

I've looked through each commit and everything looks good 👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants