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

Add x.py option to --force-rerun compiletest tests #87744

Merged
merged 1 commit into from
Aug 7, 2021

Conversation

syvb
Copy link
Contributor

@syvb syvb commented Aug 3, 2021

This can be used like ./x.py test src/test/ui/abi/ --force-rerun, and is useful when verifying that newly blessed tests don't change between test runs (such as due to being dependent on the current time or memory layout or RNG), without needing to change the test file or find the right file in build to remove.

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 3, 2021
@rust-log-analyzer

This comment has been minimized.

@Mark-Simulacrum
Copy link
Member

useful when verifying that newly blessed tests don't change between test runs (such as due to being dependent on the current time or memory layout or RNG)

I am ... a little surprised that this is something people are doing; can you share more details perhaps? I would've expected to see that under some kind of reproducible builds style environment, where the test invocation is extracted and run separately under various users, times, environment variables, etc.

It seems like there's been a lot of positive feedback from folks though based on the GitHub emojis, so I'd like to hear more but not strictly opposed to landing this. I do want to make sure that the use case is being correctly addressed, however. It may for example make more sense for x.py clean src/test/ui to delete the test artifacts and allow tests to rerun, without needing to propagate options into compile test to force a lack of caching.

@syvb
Copy link
Contributor Author

syvb commented Aug 6, 2021

@Mark-Simulacrum Often I see things in test .stderr files that looks like it is random (e.g. because it kinda looks like part of a memory address), so I manually re-run that test to check if it really is. It's not a substitute for testing on multiple platforms and environments, but it's easier to test by repeating it on the same machine first. Currently what I do when I want this is to find the right file in build to delete (I often forget where it is and have to look it up), then re-run the test I want.

@Mark-Simulacrum
Copy link
Member

Hm. Interesting! I don't think I've ever seen anything that looks like a memory address or similar in a stderr file, so I'd be interested in further details. I'm going to go ahead and r=me on this, but mark it as S-waiting-on-team because I want to hear if this is a common problem -- it might be that we should (for example) automatically run tests twice in --bless mode if we've saved a new version of the input and report differences to the user.

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 6, 2021
@syvb
Copy link
Contributor Author

syvb commented Aug 6, 2021

@Mark-Simulacrum One thing that kinda looks like a memory address I've seen is things like this. 70c9 isn't long enough to be an address, but it does look like it might be part of one, or somehow volatile. (but it is not volatile, and doesn't change between runs)

= note: required because it appears within the type `Opaque(DefId(0:39 ~ generator_print_verbose_1[70c9]::make_gen2::{opaque#0}), [std::sync::Arc<std::cell::RefCell<i32>>])`

@jyn514
Copy link
Member

jyn514 commented Aug 7, 2021

I use this for debugging what the compiler is doing, not for actually running tests. I set RUSTDOC_LOG when running tests a lot and it can be difficult to replicate UI tests through rustup, especially when they use aux-build files.

@Mark-Simulacrum
Copy link
Member

@bors r+

Seems OK, looks like we have several usage patterns at least.

@bors
Copy link
Contributor

bors commented Aug 7, 2021

📌 Commit b7e9b1a has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Aug 7, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Aug 7, 2021
…rk-Simulacrum

Add x.py option to --force-rerun compiletest tests

This can be used like `./x.py test src/test/ui/abi/ --force-rerun`, and is useful when verifying that newly blessed tests don't change between test runs (such as due to being dependent on the current time or memory layout or RNG), without needing to change the test file or find the right file in `build` to remove.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 7, 2021
Rollup of 6 pull requests

Successful merges:

 - rust-lang#87744 (Add x.py option to --force-rerun compiletest tests)
 - rust-lang#87789 (Make vec-shrink-panic test compatible with v0 mangling)
 - rust-lang#87833 (Fix typo -- "The" -> "They")
 - rust-lang#87834 (Fix small typo)
 - rust-lang#87838 (Document that fs::read_dir skips . and ..)
 - rust-lang#87842 (Fix intra doc link in hidden doc of Iterator::__iterator_get_unchecked)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e862383 into rust-lang:master Aug 7, 2021
@rustbot rustbot added this to the 1.56.0 milestone Aug 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants