Add support for resetting files

[hswolff]

Hello! It's me again :)

After #8 lands we'll have support for including test harness files for each individual process used to run tests. For my use cases these test harness files have some generic before and after handlers that I expect to run for each individual test file that I'm running.

However I encountered an issue when attempting to integrate this runner with our tests at Mongo. When this runner spins up its test runner processes it recycles them for each individual test. Meaning that test A and test C are ran within process 1 and test B and test D are ran in process 2.

However the test harness files are only ran once, when the process is first created. This means that for test A and test B the before and after handlers are ran, however because test C and D are ran within the existing processes, and the test harness files are already cached, they do not get the test harness handlers.

I have a solution that I'm using currently that I'd love to get upstreamed but it's a little more controversial IMO and wanted to discuss it first before opening a PR.

The solution is available for viewing here. The clearModuleCache is copy and pasted from mocha directly so it keeps parity there as well.

Questions:

• Is this a welcome addition?
  I hope so!
• Should it be an opt-in?
  I am leaning towards no, it should be the default behavior to simulate mocha's behavior.
• What should the option name be?
  resetFile? I'm not sure about this one. Naming is hard :(.

[rogeliog]

This is really interesting, when does purge gets called in Mocha?

[hswolff]

It's called here, when you're in 'watch' mode and you trigger a re-run of the mocha test suite.

[rogeliog]

I think this makes sense for me, but I would love to see what @ljharb thinks

[ljharb]

I'm a bit confused; why would the setup files be run more than once per suite?

If you have logic you need run before all tests, your setup file would use a beforeAll callback, no?

[hswolff]

I believe beforeAll will run before each describe block. I can create a test case to verify this.

The behavior i want is to have my test harness code ran once before all tests and once after all tests. With beforeEach I was seeing the code ran more than desired.

[ljharb]

beforeAll should run once, before the entire suite - not just before each describe. beforeEach of course runs before each it.

However, with jest-runner-mocha, if there's N files, there's N suites - so beforeAll would get run before each one (which might happen to have one describe per file). Since each file runs in its own process, this would be necessary for setting up polyfills/shims/mocks/etc.

If what you want is something that runs before/after the entire jest-run suite, I think that'd need to be a separate option unique to this runner.

[hswolff]

Well explained!

Indeed, what I want is something ran before/after either every suite ran within an individual process, or even before/after an individual suite run.

I did some more debugging and it seems like the current state of code results in some unexpected behavior (at least to me).

With the addition of the cliOptions.file option we can now include test harness code for mocha suites. i.e. I can have a testSafeguards.js file passed into cliOptions.file which contains:

before(function() {
  // do stuff before all tests
  console.log('before');
});
let count = 1;
beforeEach(function() {
  // before each test 
  console.log('beforeEach', count++);
});
after(function() {
  // after all tests
  console.log('after');
});

When I run use this file and run it before two individual test files (each being a test suite with one top level describe block) I get:

When ran with mocha directly

before
  js/common/components/ActivityFeed/ActivityDescription
beforeEach 1
    ✓ displays the right template string per activity type

  common/components/ActivityFeedTable
    when rendered with defaultProps
beforeEach 2
      ✓ shows the expected 3 columns
beforeEach 3
      ✓ shows 1 rows
beforeEach 4
      ✓ contains a dropdown filter in the first header column
beforeEach 5
      ✓ does not show the empty state message
      when row for global only users
beforeEach 6
        ✓ shows a globalOnly tooltip
      when row for isMmsAdmin
beforeEach 7
        ✓ shows a globalOnly tooltip
    when rendered with no activities
beforeEach 8
      ✓ shows the expected 3 columns
beforeEach 9
      ✓ shows 0 rows
beforeEach 10
      ✓ shows the empty state message

after

  10 passing (471ms)

When ran with `CI=true jest --verbose --maxWorkers=1` (maxWorkers 1 to force reuse of the same process)

before
beforeEach 1
beforeEach 2
beforeEach 3
beforeEach 4
beforeEach 5
beforeEach 6
beforeEach 7
beforeEach 8
beforeEach 9
after
PASS test/common/components/ActivityFeed/ActivityFeedTable.spec.js
  ✓ shows the expected 3 columns (0ms)
  ✓ shows 1 rows (0ms)
  ✓ contains a dropdown filter in the first header column (0ms)
  ✓ does not show the empty state message (0ms)
  ✓ shows a globalOnly tooltip (0ms)
  ✓ shows a globalOnly tooltip (0ms)
  ✓ shows the expected 3 columns (0ms)
  ✓ shows 0 rows (0ms)
  ✓ shows the empty state message (0ms)

PASS test/common/components/ActivityFeed/ActivityDescription.spec.js
  ✓ displays the right template string per activity type (0ms)

Test Suites: 2 passed, 2 total
Tests:       10 passed, 10 total
Snapshots:   0 total
Time:        6.94s

When I include this proposed change to delete the nodejs cache this is the output I get from jest.

Output when deleting nodejs cache

before
beforeEach 1
beforeEach 2
beforeEach 3
beforeEach 4
beforeEach 5
beforeEach 6
beforeEach 7
beforeEach 8
beforeEach 9
after
PASS test/common/components/ActivityFeed/ActivityFeedTable.spec.js
  ✓ shows the expected 3 columns (0ms)
  ✓ shows 1 rows (0ms)
  ✓ contains a dropdown filter in the first header column (0ms)
  ✓ does not show the empty state message (0ms)
  ✓ shows a globalOnly tooltip (0ms)
  ✓ shows a globalOnly tooltip (0ms)
  ✓ shows the expected 3 columns (0ms)
  ✓ shows 0 rows (0ms)
  ✓ shows the empty state message (0ms)

globalTestSafeguards
before
beforeEach 1
after
PASS test/common/components/ActivityFeed/ActivityDescription.spec.js
  ✓ displays the right template string per activity type (0ms)

Test Suites: 2 passed, 2 total
Tests:       10 passed, 10 total
Snapshots:   0 total
Time:        8.138s

It appears that due to the test harness file testSafeguards.js having been already required() for the first test file, when the second one is require'd the testSafeguards.js test hooks are not re-created as the file is already cached.

This change will resolve that issue and fix this issue.

[hswolff]

Would love to get this included upstream!