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

Implement hit counter in the mocha-runner #3193

Merged
merged 6 commits into from
Oct 15, 2021

Conversation

fredericbonnet
Copy link
Contributor

This PR fixes issue #3172 :

  • Test code is adapted from the relevant karma-runner tests
  • All tests pass on mocha-runner (NPM script test in package mocha-runner)
  • Test and E2E suites pass on parent project (NPM scripts test and e2e respectively)

The only issue that I could not solve is Mocha not detecting loop timeout in the integration test suite (file packages/mocha-runner/test/integration/timeout-on-infinite-loop.it.spec.ts, the offending test is marked as .skip). I tried changing the timeout options at many places (test, runner, options file, command line, etc) but to no avail. Despite that, the overall behavior seems correct.

@nicojs
Copy link
Member

nicojs commented Oct 12, 2021

Thanks! Will have an look later this week.

@nicojs
Copy link
Member

nicojs commented Oct 15, 2021

The only issue that I could not solve is Mocha not detecting loop timeout in the integration test suite (file packages/mocha-runner/test/integration/timeout-on-infinite-loop.it.spec.ts, the offending test is marked as .skip)

I had a look and found the issue here. I saw you copy pasted the test from the karma-runner package. The karma runner has a feature in which it is able to survive from infinite loops. It detects them and restarts the browser. However, the mocha test runner is less fortunate, since it runs the test in its own nodejs process. That's why, for mocha, we rely on the TimeoutDecorator to provide the restart functionality.

I simply removed that test.

@fredericbonnet
Copy link
Contributor Author

fredericbonnet commented Oct 15, 2021

The only issue that I could not solve is Mocha not detecting loop timeout in the integration test suite (file packages/mocha-runner/test/integration/timeout-on-infinite-loop.it.spec.ts, the offending test is marked as .skip)

I had a look and found the issue here. I saw you copy pasted the test from the karma-runner package. The karma runner has a feature in which it is able to survive from infinite loops. It detects them and restarts the browser. However, the mocha test runner is less fortunate, since it runs the test in its own nodejs process. That's why, for mocha, we rely on the TimeoutDecorator to provide the restart functionality.

I simply removed that test.

I guess there's a similar issue with Jasmine (see the other PR #3199 )

@nicojs nicojs enabled auto-merge (squash) October 15, 2021 13:13
@nicojs
Copy link
Member

nicojs commented Oct 15, 2021

I guess there's a similar issue with Jasmine (see the other PR #3199 )

Ah yes. JavaScript is single threaded. Meaning an infinite loop will block everything. Even mocha's timeout mechanism doesn't have a way of handling them.

The only reason karma is able to handle them is because karma starts the tests in a separate process (the browser). All other test runners we support use the host nodejs process to run the tests (even jest, because we run with --runInBand). This is why we have the timeout decorator in Stryker's core.

The timeout decorator also works for Karma, but because Karma can be very slow to restart, we've added additional handling in the karma-runner 🤷‍♀️.

@nicojs nicojs force-pushed the feat/hit-counter-mocha branch from 5b2e5b3 to 4abcdca Compare October 15, 2021 13:52
@nicojs nicojs merged commit f5a7d1d into stryker-mutator:master Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants