Fix up-to-date checking for run-make tests #131681

Zalathar · 2024-10-14T11:44:59Z

This special case in output_base_dir had the unfortunate side-effect of causing all run-make tests to share the same stamp file. So as soon as any one of them succeeded, all of the failed tests would be incorrectly considered up-to-date and would no longer run in subsequent test invocations.

Fixes #129971.

r? jieyouxu

rustbot · 2024-10-14T11:45:08Z

Some changes occurred in src/tools/compiletest

cc @jieyouxu

Zalathar · 2024-10-14T11:46:20Z

Reverts part of #127958.

The special-case was added to eliminate an extra level of nesting in the run-make directories, so this fix necessarily reverses that, unfortunately.

jieyouxu · 2024-10-14T11:50:10Z

Thanks, could you also update

rust/tests/run-make/README.md

Lines 27 to 35 in f6648f2

    
           2. Then, we compile the `rmake.rs` "recipe" linking the support library and its 
        
              dependencies in, and provide a bunch of env vars. We setup a directory 
        
              structure within `build/<target>/test/run-make/` 
        
              ``` 
        
              <test-name>/ 
        
                  rmake.exe              # recipe binary 
        
                  rmake_out/             # sources from test sources copied over 
        
              ```

to reflect the double-nesting?

jieyouxu

Yeah, double nesting is unfortunate, but incorrect ignore test logic is even worse.

This special case in `output_base_dir` had the unfortunate side-effect of causing all run-make tests to share the same `stamp` file. So as soon as any one of them succeeded, all of the failed tests would be considered up-to-date and would no longer run in subsequent test invocations.

rustbot · 2024-10-14T11:56:16Z

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

jieyouxu

Thanks, r=me after PR CI is green.

Zalathar · 2024-10-14T12:04:19Z

For reference: I think the extra nesting can be fixed by making run_rmake_v2_test call self.output_base_dir() instead of self.output_base_name().

But given the subtle trouble caused by the last fix, I want to think about this one a bit more before actually making the change.

jieyouxu · 2024-10-14T12:21:28Z

I would prefer we keep this PR simple to fix the incorrect up-to-date checking (which is more important). Improvements to flatten the nesting could be a follow-up as we may find ourselves needing to revert that change, lol.

Zalathar · 2024-10-14T12:26:25Z

Oh yes, to be clear I have no intention of making that change in this PR.

Zalathar · 2024-10-14T13:43:53Z

🟩

@bors r=jieyouxu rollup

bors · 2024-10-14T13:43:56Z

📌 Commit c6e1fbf has been approved by jieyouxu

It is now in the queue for this repository.

…youxu Fix up-to-date checking for run-make tests This special case in `output_base_dir` had the unfortunate side-effect of causing all run-make tests to share the same `stamp` file. So as soon as any one of them succeeded, all of the failed tests would be incorrectly considered up-to-date and would no longer run in subsequent test invocations. Fixes rust-lang#129971. r? jieyouxu

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#122670 (Fix bug where `option_env!` would return `None` when env var is present but not valid Unicode) - rust-lang#130568 (Make some float methods unstable `const fn`) - rust-lang#131137 (Add 1.82 release notes) - rust-lang#131328 (Remove unnecessary sorts in `rustc_hir_analysis`) - rust-lang#131652 (Move polarity into `PolyTraitRef` rather than storing it on the side) - rust-lang#131675 (Update lint message for ABI not supported) - rust-lang#131681 (Fix up-to-date checking for run-make tests) - rust-lang#131703 (Resolved python deprecation warning in publish_toolstate.py) - rust-lang#131706 (Fix two const-hacks) r? `@ghost` `@rustbot` modify labels: rollup

…youxu Fix up-to-date checking for run-make tests This special case in `output_base_dir` had the unfortunate side-effect of causing all run-make tests to share the same `stamp` file. So as soon as any one of them succeeded, all of the failed tests would be incorrectly considered up-to-date and would no longer run in subsequent test invocations. Fixes rust-lang#129971. r? jieyouxu

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#122670 (Fix bug where `option_env!` would return `None` when env var is present but not valid Unicode) - rust-lang#131095 (Use environment variables instead of command line arguments for merged doctests) - rust-lang#131339 (Expand set_ptr_value / with_metadata_of docs) - rust-lang#131652 (Move polarity into `PolyTraitRef` rather than storing it on the side) - rust-lang#131675 (Update lint message for ABI not supported) - rust-lang#131681 (Fix up-to-date checking for run-make tests) - rust-lang#131702 (Suppress import errors for traits that couldve applied for method lookup error) - rust-lang#131703 (Resolved python deprecation warning in publish_toolstate.py) - rust-lang#131710 (Remove `'apostrophes'` from `rustc_parse_format`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#131681 - Zalathar:fix-run-make-stamp, r=jieyouxu Fix up-to-date checking for run-make tests This special case in `output_base_dir` had the unfortunate side-effect of causing all run-make tests to share the same `stamp` file. So as soon as any one of them succeeded, all of the failed tests would be incorrectly considered up-to-date and would no longer run in subsequent test invocations. Fixes rust-lang#129971. r? jieyouxu

Run-make tests were using `output_base_name` to determine their output directory, which results in a redundant subdirectory (e.g. `$build/test/run-make/<foo>/<foo>/`) because that method is intended to produce the name of an individual file. The previous attempt to fix this double-nesting tried adding a special case in `output_base_dir`, which had the side-effect of breaking up-to-date checking for run-make tests, and had to be reverted in rust-lang#131681. The fix is simply to call `output_base_dir` directory, which gives the desired directory without any redundant part.

Fix unnecessary nesting in run-make test output directories Run-make tests were using `output_base_name` to determine their output directory, which results in a redundant subdirectory (e.g. `$build/test/run-make/<foo>/<foo>/`) because that method is intended to produce the name of an individual file. The previous attempt to fix this double-nesting tried adding a special case in `output_base_dir`, which had the side-effect of breaking up-to-date checking for run-make tests, and had to be reverted in rust-lang#131681. The fix is simply to call `output_base_dir` directory, which gives the desired directory without any redundant part. r? jieyouxu

Rollup merge of rust-lang#131764 - Zalathar:double-dir, r=jieyouxu Fix unnecessary nesting in run-make test output directories Run-make tests were using `output_base_name` to determine their output directory, which results in a redundant subdirectory (e.g. `$build/test/run-make/<foo>/<foo>/`) because that method is intended to produce the name of an individual file. The previous attempt to fix this double-nesting tried adding a special case in `output_base_dir`, which had the side-effect of breaking up-to-date checking for run-make tests, and had to be reverted in rust-lang#131681. The fix is simply to call `output_base_dir` directory, which gives the desired directory without any redundant part. r? jieyouxu

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#122670 (Fix bug where `option_env!` would return `None` when env var is present but not valid Unicode) - rust-lang#131095 (Use environment variables instead of command line arguments for merged doctests) - rust-lang#131339 (Expand set_ptr_value / with_metadata_of docs) - rust-lang#131652 (Move polarity into `PolyTraitRef` rather than storing it on the side) - rust-lang#131675 (Update lint message for ABI not supported) - rust-lang#131681 (Fix up-to-date checking for run-make tests) - rust-lang#131702 (Suppress import errors for traits that couldve applied for method lookup error) - rust-lang#131703 (Resolved python deprecation warning in publish_toolstate.py) - rust-lang#131710 (Remove `'apostrophes'` from `rustc_parse_format`) r? `@ghost` `@rustbot` modify labels: rollup

Zalathar added the A-compiletest Area: The compiletest test runner label Oct 14, 2024

rustbot assigned jieyouxu Oct 14, 2024

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Oct 14, 2024

jieyouxu approved these changes Oct 14, 2024

View reviewed changes

Zalathar force-pushed the fix-run-make-stamp branch from beff457 to c6e1fbf Compare October 14, 2024 11:56

rustbot added the A-run-make Area: port run-make Makefiles to rmake.rs label Oct 14, 2024

jieyouxu approved these changes Oct 14, 2024

View reviewed changes

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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 14, 2024

matthiaskrgr mentioned this pull request Oct 14, 2024

Rollup of 9 pull requests #131709

Closed

matthiaskrgr mentioned this pull request Oct 14, 2024

Rollup of 9 pull requests #131711

Closed

compiler-errors mentioned this pull request Oct 15, 2024

Rollup of 10 pull requests #131722

Closed

matthiaskrgr mentioned this pull request Oct 15, 2024

Rollup of 9 pull requests #131723

Merged

bors merged commit 258c177 into rust-lang:master Oct 15, 2024
6 checks passed

rustbot added this to the 1.84.0 milestone Oct 15, 2024

Zalathar deleted the fix-run-make-stamp branch October 15, 2024 22:12

Zalathar mentioned this pull request Oct 16, 2024

Fix unnecessary nesting in run-make test output directories #131764

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix up-to-date checking for run-make tests #131681

Fix up-to-date checking for run-make tests #131681

Zalathar commented Oct 14, 2024 •

edited by jieyouxu

Loading

rustbot commented Oct 14, 2024

Zalathar commented Oct 14, 2024

jieyouxu commented Oct 14, 2024

jieyouxu left a comment

rustbot commented Oct 14, 2024

jieyouxu left a comment

Zalathar commented Oct 14, 2024

jieyouxu commented Oct 14, 2024

Zalathar commented Oct 14, 2024

Zalathar commented Oct 14, 2024

bors commented Oct 14, 2024

Fix up-to-date checking for run-make tests #131681

Fix up-to-date checking for run-make tests #131681

Conversation

Zalathar commented Oct 14, 2024 • edited by jieyouxu Loading

rustbot commented Oct 14, 2024

Zalathar commented Oct 14, 2024

jieyouxu commented Oct 14, 2024

jieyouxu left a comment

Choose a reason for hiding this comment

rustbot commented Oct 14, 2024

jieyouxu left a comment

Choose a reason for hiding this comment

Zalathar commented Oct 14, 2024

jieyouxu commented Oct 14, 2024

Zalathar commented Oct 14, 2024

Zalathar commented Oct 14, 2024

bors commented Oct 14, 2024

Zalathar commented Oct 14, 2024 •

edited by jieyouxu

Loading