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

mocha-runner: Add support for test's fileName #3504

Merged
merged 5 commits into from
Jun 26, 2022

Conversation

IvanGoncharov
Copy link
Contributor

Mocha support filenames, see: mochajs/mocha@3dd1cdb

I plan to use it as my plugin.
I tried to write tests based on jest-runner tests from #2808 but I'm lost in the test code.
I'm interested in getting this PR merged so would be grateful if someone adopts this PR and write missing tests.

@nicojs
Copy link
Member

nicojs commented May 17, 2022

Hi @IvanGoncharov 🙋‍♂️

This is actually pretty awesome! I didn't even know this feature was added, it seems to be missing from the changelog

Adding this file is a nice first step, however, it probably needs some more love. I think we should add automatic source map resolution.

I'll try to find some time to test this out and see how it works without sourcemap. If it can already deliver some value I'll merge this and work on sourcemaps in another branch.

EDIT: Even more important is the location within the file. That is still missing. See mochajs/mocha#4572

@nicojs
Copy link
Member

nicojs commented May 18, 2022

Interesting! This is what Stryker makes of it:

mocha-test-files

As you can see it kind of works. For Stryker it shows the *.js files instead of the source files, as expected. The tests are shown in one big list since the location inside the file is missing.

I personally think this is already adding value. What do you think @IvanGoncharov?

(report here: mutation.zip)

@IvanGoncharov
Copy link
Contributor Author

I personally think this is already adding value. What do you think @IvanGoncharov?

@nicojs yes, it adds value for me.

Actually, I want to share context on what I'm currently doing and maybe you can suggest some additional tips.

I recently adopted stryker on https://github.com/graphql/graphql-js and we got 90% mutant coverage 🎉
Ideally, I like to run it for every PR but currently, it takes between 3 and 5 hours for a complete run.
So I like to try an optimization strategy based on the test structure in our project.

We have __tests__ dir in every folder with source code.
So I want to try optimization where I sort tests based on how likely they are to kill the mutant.
So if I have mutant for source file src/foo/bar.ts here is a priority for tests based on their source file:

  1. src/foo/__tests__/bar-test.ts
  2. src/foo/__tests__/*-test.ts
  3. src/**/__tests__/*-test.ts

I'm betting on the fact that most of the mutants will be killed by src/foo/__tests__/bar-test.ts which mean only a few dozens of test gets run per mutant.

It looks like MutantPlanner has all the necessary info.

MutantPlanner also has access to execution time for every test so I want to try the second assumption "it's better to run faster tests first". So I first want to sort tests by execution speed and later sort them based on file path (as shown above).

Currently, my biggest problem is that mocha doesn't allow you to sort tests and only allows you to filter them using RegExp without changing their order :(

Not sure, if those experiments will work or how useful for other projects they will end up.
But I'm slowly working on it as a hobby project with a plan to upstream stuff that makes sense in a core.

@nicojs
Copy link
Member

nicojs commented Jun 24, 2022

Yeah, test sorting would be a nice performance boost for some projects. It would need an implementation specific to each test runner, bringing a big maintenance overhead. And most test runners don't support a custom order (I think jasmine and jest are the only ones).

@nicojs
Copy link
Member

nicojs commented Jun 26, 2022

I'll try to get this PR merged today. For the sorting of tests, feel free to open a separate issue. We could implement it for test runners that support it.

For jest, we could create a custom sequencer
https://jestjs.io/docs/configuration#testsequencer-string

@nicojs
Copy link
Member

nicojs commented Jun 26, 2022

Example of the clear-text reporter output:

All tests
  Add.spec.js
    ✓ Add should be able to add two numbers (killed 2)
    ✓ Add should be able 1 to a number (killed 2)
    ✓ Add should be able negate a number (killed 2)
    ✓ Add should be able to recognize a negative number (killed 5)
    ✓ Add should be able to recognize that 0 is not a negative number (killed 3)
  Circle.spec.js
    ✓ Circle should have a circumference of 2PI when the radius is 1 (killed 2)
  IsEven.spec.js
    ✓ isEven should be false for 1 (killed 5)
    ✓ isEven should be true for 2 (killed 3)
  ninjaCat.spec.js
    ✓ 🐱‍👓 ninjaCatSays should speak (killed 2)

@nicojs nicojs enabled auto-merge (squash) June 26, 2022 08:33
@nicojs nicojs merged commit 34d8e70 into stryker-mutator:master Jun 26, 2022
@IvanGoncharov IvanGoncharov deleted the mochaAddFileName branch July 7, 2022 10:52
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