-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Print a helpful message if any tests were skipped for being up-to-date #130131
Conversation
When running tests without the `--force-rerun` flag, compiletest will automatically skip any tests that (in its judgement) don't need to be run again since the last time they were run. This patch adds an explicit reason to those skipped tests, which is visible when running with `rust.verbose-tests = true` in `config.toml`.
Some changes occurred in src/tools/compiletest cc @jieyouxu |
I'm hoping that this is unobtrusive enough to be worth always emitting whenever up-to-date tests are skipped, but I'm certainly open to other opinions. |
This is a good improvement, but I wonder if we could even make up-to-date tests a first class citizen, so that we could print something like this:
and to make the detection more robust (rather than checking a string in the ignore reason. I find it weird to treat up-to-date tests as ignored, it can also be misleading, e.g. if we check ignored tests on CI, if for some reason some of them were just up-to-date due to some unexpected bootstrap behavior, then we would miscount them. And in fact detecting that CI has no up-to-date tests could also be useful. @jieyouxu what do you think? |
In terms of proper first-class support for up-to-date tests, we're somewhat constrained by the fact that the test results seen by bootstrap travel through libtest as an intermediate step, so we would need to be able to propagate that extra information through libtest. I'm not sure what's involved in adding more custom unstable stuff to libtest. |
That said, I would certainly be happy to see better reporting of up-to-date tests in some form. |
Oh, I didn't realize that it's from libtest, rather than compiletest. Yeah, then probably let's not go there for now 😆 @bors r+ |
IIRC the actual status line is printed by bootstrap, based on stats reported by libtest. So we could do some arithmetic and break out the up-to-date tests from the overall ignored count, but I’m a bit wary of adding that much magic to the main status line (rather than faithfully reporting the libtest numbers). |
Aah, I was looking for the line in I don't think that we necessarily need to reproduce libtest output 1:1, but good point. Let's land your change, I think it's unobtrusive enough, and then we can discuss with others if they would be ok with making larger changes. |
Print a helpful message if any tests were skipped for being up-to-date When running tests without the `--force-rerun` flag, compiletest will automatically skip any tests that (in its judgement) don't need to be run again since the last time they were run. This is normally very useful, but can occasionally be confusing, especially in edge-cases where up-to-date checking is not completely accurate (or the test is flaky). This PR makes bootstrap count the number of tests that were ignored for being up-to-date (via a hard-coded check on the ignore reason), and prints a helpful message when that number is nonzero. --- Sample output: ```text test result: ok. 4 passed; 0 failed; 17578 ignored; 0 measured; 0 filtered out; finished in 463.79ms help: ignored 17295 up-to-date tests; use `--force-rerun` to prevent this Build completed successfully in 0:00:07 ```
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#129929 (`rustc_mir_transform` cleanups, round 2) - rust-lang#130022 (Dataflow/borrowck lifetime cleanups) - rust-lang#130064 (fix ICE in CMSE type validation) - rust-lang#130067 (Remove redundant check in `symlink_hard_link` test) - rust-lang#130131 (Print a helpful message if any tests were skipped for being up-to-date) - rust-lang#130137 (Fix ICE caused by missing span in a region error) - rust-lang#130153 (use verbose flag as a default value for `rust.verbose-tests`) - rust-lang#130154 (Stabilize `char::MIN`) - rust-lang#130158 (Update books) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#130131 - Zalathar:up-to-date, r=Kobzol Print a helpful message if any tests were skipped for being up-to-date When running tests without the `--force-rerun` flag, compiletest will automatically skip any tests that (in its judgement) don't need to be run again since the last time they were run. This is normally very useful, but can occasionally be confusing, especially in edge-cases where up-to-date checking is not completely accurate (or the test is flaky). This PR makes bootstrap count the number of tests that were ignored for being up-to-date (via a hard-coded check on the ignore reason), and prints a helpful message when that number is nonzero. --- Sample output: ```text test result: ok. 4 passed; 0 failed; 17578 ignored; 0 measured; 0 filtered out; finished in 463.79ms help: ignored 17295 up-to-date tests; use `--force-rerun` to prevent this Build completed successfully in 0:00:07 ```
Yeah this seems fine, I'm less concerned about up-to-date tests being skipped (which is intuitive to me at least), but more concerned about tests being incorrectly skipped (lol) |
When running tests without the
--force-rerun
flag, compiletest will automatically skip any tests that (in its judgement) don't need to be run again since the last time they were run.This is normally very useful, but can occasionally be confusing, especially in edge-cases where up-to-date checking is not completely accurate (or the test is flaky).
This PR makes bootstrap count the number of tests that were ignored for being up-to-date (via a hard-coded check on the ignore reason), and prints a helpful message when that number is nonzero.
Sample output: