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

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

Merged
merged 1 commit into from
Oct 16, 2024

Conversation

Zalathar
Copy link
Contributor

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 #131681.

The fix is simply to call output_base_dir directory, which gives the desired directory without any redundant part.

r? jieyouxu

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-run-make Area: port run-make Makefiles to rmake.rs 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 16, 2024
@rustbot
Copy link
Collaborator

rustbot commented Oct 16, 2024

Some changes occurred in src/tools/compiletest

cc @jieyouxu

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

@Zalathar
Copy link
Contributor Author

I tested this manually, and the directory structure and up-to-date checker appear to be working as expected.

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested two things locally:

  1. That input stamping is properly preserved. I ran ./x test run-make twice and can confirm test failures from first run (i.e. no make on windows) do not get incorrectly ignored on the subsequent run.
  2. That there is no double-layering of the test output directory.

I can confirm both of these work on locally test.

Thank you!

@jieyouxu
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 16, 2024

📌 Commit 4cf0475 has been approved by jieyouxu

It is now in the queue for this repository.

@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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 16, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 16, 2024
Rollup of 10 pull requests

Successful merges:

 - rust-lang#131582 (Add wasm32-unknown-emscripten platform support document)
 - rust-lang#131694 (Make fuchsia-test-runner.py compatible with new JSON output from llvm-readelf)
 - rust-lang#131700 (Fix match_same_arms in stable_mir)
 - rust-lang#131712 (Mark the unstable LazyCell::into_inner const)
 - rust-lang#131746 (Relax a memory order in `once_box`)
 - rust-lang#131754 (Don't report bivariance error when nesting a struct with field errors into another struct)
 - rust-lang#131760 (llvm: Match aarch64 data layout to new LLVM layout)
 - rust-lang#131764 (Fix unnecessary nesting in run-make test output directories)
 - rust-lang#131766 (Add mailmap entry for my dev-desktop setup)
 - rust-lang#131771 (Handle gracefully true/false in `cfg(target(..))` compact)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 43a142e into rust-lang:master Oct 16, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Oct 16, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 16, 2024
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
@Zalathar Zalathar deleted the double-dir branch October 16, 2024 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants