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

Support only run the initial test run (dry run) #3088

Closed
karfau opened this issue Aug 24, 2021 · 10 comments · Fixed by #3814
Closed

Support only run the initial test run (dry run) #3088

karfau opened this issue Aug 24, 2021 · 10 comments · Fixed by #3814
Labels
🚀 Feature request New feature request 👶 Good first issue Good for newcomers hacktoberfest https://hacktoberfest.digitalocean.com/

Comments

@karfau
Copy link

karfau commented Aug 24, 2021

Question

Since our stryker run takes around 2 hours to complete we currently only run it on the default branch.
It would be nice if we could still execute just the initial test run in branches to avoid that it fails on master and we need to figure out why it broke/revert...

Stryker environment

+-- @stryker-mutator/core@^5.2.2
+-- jest@^27.0.6

Additional context

@karfau karfau added the ⁉ Question Further information is requested label Aug 24, 2021
@nicojs
Copy link
Member

nicojs commented Aug 24, 2021

My hopes are that running unit tests normally is enough. You generally want to share the test runner configuration with stryker using jest.config.js, .mocharc.json, etc. That way, Stryker should have no issues running your tests.

That being said, there are still slight differences for Stryker vs your test runner. I think this will work: npx stryker run -m not-exists.js.

We might want to add a --dry-run in the future, to only run the dry run.

@karfau
Copy link
Author

karfau commented Aug 25, 2021

Ok, i just checked what -m means and it refers to https://stryker-mutator.io/docs/stryker-js/configuration#mutate-string
So it would also work yo have an additional config file with "mutate": [], right?
I wonder if we can pass an "empty glob/list" from the cli instead of a non existing file?
Most likely will experiment with this very soon.

As a side note: maybe it's worth documenting this behavior of the option? Something like

Passing a glob that matches no files will cause a "dryrun": The configuration will be read and the tests will be run once, but no files will be mutated.

(I'm very curios about the expected exit code when using this approach...)

Update: I can confirm that it works nicely and the cli output it already properly indicating what is going on and the exit code is 0:

 npm run stryker -- -m "dryrun" && echo all good

> stryker run "-m" "dryrun"

07:45:58 (102256) INFO ConfigReader Using stryker.conf.json
07:45:58 (102256) WARN InputFileResolver Glob pattern "dryrun" did not result in any files.
07:45:59 (102256) WARN InputFileResolver No files marked to be mutated, Stryker will perform a dry-run without actually mutating anything. You can configure the `mutate` property in your config file (or use `--mutate` via command line).
07:45:59 (102256) INFO Instrumenter Instrumented 0 source file(s) with 0 mutant(s)
07:45:59 (102256) INFO ConcurrencyTokenProvider Creating 7 test runner process(es).
07:45:59 (102256) INFO DryRunExecutor Starting initial test run (command test runner with "off" coverage analysis). This may take a while.
07:46:05 (102256) INFO DryRunExecutor Initial test run succeeded. Ran 1 tests in 6 seconds (net 6027 ms, overhead 2 ms).

All tests
  ✘ All tests (covered 0)

Ran NaN tests per mutant on average.
----------|---------|----------|-----------|------------|----------|---------|
File      | % score | # killed | # timeout | # survived | # no cov | # error |
----------|---------|----------|-----------|------------|----------|---------|
All files |     NaN |        0 |         0 |          0 |        0 |       0 |
----------|---------|----------|-----------|------------|----------|---------|
07:46:05 (102256) INFO HtmlReporter Your report can be found at: file:///[...]/reports/mutation/html/index.html
07:46:05 (102256) INFO MutationTestExecutor Done in 6 seconds.
all good

karfau added a commit to xmldom/xmldom that referenced this issue Aug 25, 2021
Dryrun works by passing a glob that matches no file.
Additionally we reduce the reporters since there is nothing to report anyways.
I decided to go for a CLI approach instead of a separate config file,
since there is no way to extend config files, and I would like to avoid that we need to keep the two files in sync.

stryker-mutator/stryker-js#3088
karfau added a commit to xmldom/xmldom that referenced this issue Aug 26, 2021
Dryrun works by passing a glob that matches no file.
Additionally we reduce the reporters since there is nothing to report anyways.
I decided to go for a CLI approach instead of a separate config file,
since there is no way to extend config files, and I would like to avoid that we need to keep the two files in sync.

stryker-mutator/stryker-js#3088
@brodybits
Copy link

brodybits commented Aug 26, 2021

npx stryker run -m not-exists.js.

Using -m '' works for me on macOS, at least:

brodybits@brodybits-mini-mac-book xmldom % npx stryker run -m '' --reporters progress
16:34:07 (19982) INFO ConfigReader Using stryker.conf.json
16:34:07 (19982) WARN InputFileResolver Glob pattern "" did not result in any files.
16:34:07 (19982) WARN InputFileResolver No files marked to be mutated, Stryker will perform a dry-run without actually mutating anything. You can configure the `mutate` property in your config file (or use `--mutate` via command line).
16:34:07 (19982) INFO Instrumenter Instrumented 0 source file(s) with 0 mutant(s)
16:34:08 (19982) INFO ConcurrencyTokenProvider Creating 4 test runner process(es).
16:34:08 (19982) INFO DryRunExecutor Starting initial test run (command test runner with "off" coverage analysis). This may take a while.
16:34:37 (19982) INFO DryRunExecutor Initial test run succeeded. Ran 1 tests in 28 seconds (net 28741 ms, overhead 2 ms).
16:34:37 (19982) INFO MutationTestExecutor Done in 30 seconds.

+1 (+100) for getting this dry run only behavior more explicitly documented in case it may help any others.


P.S. Getting rid of the --reporters option results in some extra error output on the console which is not needed & not wanted.

@nicojs
Copy link
Member

nicojs commented Aug 27, 2021

I thought about this approach, and it is not a "true dry run". Using -m '' will configure Stryker to not mutate any files, so they also wouldn't be instrumented. This means that bugs like #3080 could still creep in when running on the master branch.

With all of that being said, I think we should support a --dry-run option after all. Or maybe a stryker dry-run command.

@karfau
Copy link
Author

karfau commented Aug 27, 2021

We are currently having an update of @stryker/core in our pipeline that we needed to revert because the initial test run failed on master.

After reading your comment I will check if the dryrun would actually fail or only the "instrumented" run fails. I will update this comment when I know the result.

(I'm not trying to talk about why it fails here, if required I will file a new issue for that. It's just to support the case for a proper dry-run.)

karfau added a commit to xmldom/xmldom that referenced this issue Aug 27, 2021
Dryrun works by passing a glob that matches no file.
Additionally we reduce the reporters since there is nothing to report anyways.
I decided to go for a CLI approach instead of a separate config file,
since there is no way to extend config files, and I would like to avoid that we need to keep the two files in sync.

stryker-mutator/stryker-js#3088

revert updates to .github/workflows/stryker.yml
brodybits added a commit to xmldom/xmldom that referenced this issue Aug 27, 2021
Dry run works by passing a glob that matches no file.
Additionally we reduce the reporters since there is nothing to report anyways.

We decided to go for a CLI approach instead of a separate config file,
since there is no way to extend config files, and we would like to avoid
any need to keep the two files in sync.

stryker-mutator/stryker-js#3088

Co-authored-by: Christian Bewernitz <coder@karfau.de>
Co-authored-by: Christopher J. Brody <chris.brody+brodybits@gmail.com>
@karfau
Copy link
Author

karfau commented Sep 1, 2021

I can confirm:
When updating from the stryker version mentioned above to 5.3.0 or 5.4.0 the dry-run proposed here is green, but on the real run the initial test run fails.

(It's caused by stryker adding throw new Error('Stryker: ... into the instrumented code and our tests indexing all errors being thrown, but now the indexes do no longer match. I was able to fix that part on our side.)

@nicojs
Copy link
Member

nicojs commented Sep 2, 2021

Great, that's what I was afraid of. Please open an issue for the instrumentation error, we'll use this issue to track progress on the dry-run feature

@nicojs nicojs added 🚀 Feature request New feature request 👶 Good first issue Good for newcomers and removed ⁉ Question Further information is requested labels Sep 2, 2021
@nicojs nicojs changed the title Is it possible to only run the initial test run? Support only run the initial test run (dry run) Sep 2, 2021
@karfau
Copy link
Author

karfau commented Sep 2, 2021

(This will be the last off topic message, feel free to hide after reading!)

I would not file an issue about it, since I'm not sure what to request as a "fix". I think it's fine for Stryker to throw an Error as part of the instrumentation if it needs to. Our test setup was just very special.

@stale
Copy link

stale bot commented Sep 4, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the ☠ stale Marked as stale by the stale bot, will be removed after a certain time. label Sep 4, 2022
@stale stale bot closed this as completed Oct 8, 2022
@nicojs
Copy link
Member

nicojs commented Oct 9, 2022

We probably still want this feature.

@nicojs nicojs reopened this Oct 9, 2022
@nicojs nicojs added hacktoberfest https://hacktoberfest.digitalocean.com/ and removed ☠ stale Marked as stale by the stale bot, will be removed after a certain time. labels Oct 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 Feature request New feature request 👶 Good first issue Good for newcomers hacktoberfest https://hacktoberfest.digitalocean.com/
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants