-
Notifications
You must be signed in to change notification settings - Fork 127
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
Rerun fails with coverage and "dependent" subtests #274
Comments
Thanks for opening this issue!
There is a hidden flag
That's an interesting problem. I think you'd have to solve this by using # test-with-coverage.sh
#!/usr/bin/env bash
set -eu
go test -json -coverprofile=tmp-coverage.out -coverpkg=. "$@"
cat tmp-coverage.out >> coverage.out That should accept the extra args from That can be used like so:
I haven't tried it myself, but something like this should work. |
I opened #275 which should address the first request. I renamed the flag to |
@dnephin Thank you very much for you detailed answer and the PR. I highly appreciate it! I've commented in #275 already that it solves the part concerning dependent subtests. Concerning the coverage, your idea of con Still, the above solution with a |
I guess something like this might also work to remove those extra lines? go test -json -coverprofile=tmp-coverage.out -coverpkg=. "$@"
grep -v 'mode: set' tmp-coverage.out >> coverage.out Although maybe you'd then have to re-add the line once as well.
I am not sure. Coverage is not something I use much myself, so I'm unlikely to add it. I'm not entirely sure how If you have some idea of how this would look (ex: what command line flags need to be added, what values would they accept, and how would it work at a high level) I'd be happy to review the design and try and work with you to find a clean solution. |
Looking at the implementation of gocovmerge I think it's a bit more involved than simply fiddling with The only way I could think of a meaningful integration would be that I could try creating a PR for that, I've looked into |
@dnephin Guess you're busy with other things, but assuming I'd find time to look into this, is there a chance you'd merge such a feature? |
I'd like to help you find a solution to this problem, but I'm not keen on maintaining a feature that requires knowledge of the coverage profile (even if it calls out to I looked at
I would expect a coverage profile generated with Why isn't simply concatenating the lines together sufficient? Is the tool that reads the profile not able to combine duplicate lines? I found this old post https://lk4d4.darth.io/posts/multicover/ written by a former colleague of mine. It sounds like concatenating the profiles may be fine for the I'd much rather solve the problem with a script and Maybe something like this would work to use test-with-coverage.sh #!/usr/bin/env bash
set -eu
go test -json -coverprofile=tmp-coverage-${RANDOM}.out -coverpkg=. "$@" That will generate the coverage profile to a file with a random name each time. That should let us use gotestsum --rerun-fails --raw-command ./test-with-coverage.sh
gocovmerge tmp-coverage-*.out > coverage.out
rm tmp-coverage-*.out
go tool cover -func coverage.out If that works correct with From there we could look at making it easier to integrate. Maybe I really want to avoid having At the very least it would be great to document how to get this working together with |
If it doesn't work that way it's possible that the logic for merging profiles will be involved than what It's possible what we need is instead a way to only gather coverage from the first run, and to omit the |
@dnephin Thanks for your very detailed reply. I fear you might have missed my quoted demo already above, which shows that the solution you've been proposing pretty much works as expected. Or am I mistaken and you had another point? |
Ah, yes I did miss that, I had not realized you had it all working! That's awesome! So the problem left to solve is making it easier to use, is that right? If I'm not sure what the best way to do that would be. Some options that come to mind:
If there were any other |
Has there been any update on this? |
@ofek I've somewhat lost interest in it as I've solved rerunning tests differently using Gitlab CI retries. Also a fully worked out workaround is available. |
gotestyourself/gotestsum#274 Updates gotestsum to v1.11.0
gotestyourself/gotestsum#274 Updates gotestsum to v1.11.0
gotestyourself/gotestsum#274 Updates gotestsum to v1.11.0
gotestyourself/gotestsum#274 Updates gotestsum to v1.11.0
gotestyourself/gotestsum#274 Updates gotestsum to v1.11.0
gotestyourself/gotestsum#274 Updates gotestsum to v1.11.0
When `--rerun-fails` is specified and a test is re-run, the produced coverage data only contains the coverage from the failed but re-executed sub-tests. See gotestyourself/gotestsum#274 This change sees how we fare with this disabled as we've recently done some work to address test flakiness. We'll be relying on the much less granular `./scripts/retry` for retries, which doesn't have the coverage problem. The downside is it will re-run all the tests.
I've started to use the
--rerun-fails
option and noticed that when coverage profiling is enabled, the producedcoverage.out
file only contains the coverage from the failed but re-executed sub-tests.For example:
The above example usage is from this little demo project to illustrate my issue, see
run-test.sh
script there.I've tried to catch the
coverage.out
from the first run using with--post-run-command="bash -c 'mv coverage.out $(mktemp coverage.out.XXXXXXXXXX)'"
, but that only produced one file. I assume that's because the post-run command is only executed after all re-runs are done by gotestsum.Do you have a suggestion how to calculate coverage correctly and/or generate a "merged" coverage.out file from all re-runs?
Futhermore, I'd like to see a command line option such as
--rerun-all-subtests
which re-runs not only one specific failed sub-test, but essentially all tests of the test case containing the failed sub-test (i.e. find the root of a failed subtest separated by/
and run that one again). That would help when a sophisticated integration test is split up into sub-tests and subsequent sub-tests need all previous sub-tests to be run to be successful. I can also prepare an illustration to this problem, but I hope you get the idea. What do you think?The text was updated successfully, but these errors were encountered: