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

Remove extra removal from test path #107027

Merged
merged 1 commit into from
Jan 19, 2023

Conversation

GuillaumeGomez
Copy link
Member

I don't know how to describe it shortly so better show what it's doing instead. Currently, there is one extra "rust/" before the test folder when running tests:

failures:

---- [rustdoc] rust/tests/rustdoc/redirect.rs stdout ----

This is a bit annoying when copying the test path. This is due to the moving of the tests folder one level up, meaning we were trimming too much of the root_path.

Now it is again displaying the correct path:

failures:

---- [rustdoc] tests/rustdoc/redirect.rs stdout ----

@rustbot
Copy link
Collaborator

rustbot commented Jan 18, 2023

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@rustbot 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. labels Jan 18, 2023
Copy link
Member

@albertlarsan68 albertlarsan68 left a comment

Choose a reason for hiding this comment

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

I should have verified all of the outputs.

Is there any other instance of this .parent().unwrap() scheme ?

@GuillaumeGomez
Copy link
Member Author

There are a few places but they don't seem to impact the tests folder:

library/std/src/path.rs:2279:    /// `&self.parent().unwrap().parent().unwrap()` and so on. If the [`parent`] method returns
src/bootstrap/config.rs:819:        config.src = manifest_dir.parent().unwrap().parent().unwrap().to_owned();
src/bootstrap/test.rs:1729:                .arg(builder.cc(target).parent().unwrap().parent().unwrap());
src/tools/compiletest/src/runtest.rs:2933:        let src_root = self.config.src_base.parent().unwrap().parent().unwrap();
src/tools/compiletest/src/runtest.rs:3550:            Some(self.config.src_base.parent().unwrap().parent().unwrap().into()),
src/tools/compiletest/src/runtest.rs:3573:        let parent_build_dir = test_build_dir.parent().unwrap().parent().unwrap().parent().unwrap();
src/tools/rust-analyzer/crates/rust-analyzer/src/cli/load_cargo.rs:156:        let path = Path::new(env!("CARGO_MANIFEST_DIR")).parent().unwrap().parent().unwrap();
src/tools/rust-analyzer/crates/sourcegen/src/lib.rs:199:    let res = PathBuf::from(dir).parent().unwrap().parent().unwrap().to_owned();
src/tools/rust-analyzer/crates/test-utils/src/lib.rs:407:    PathBuf::from(dir).parent().unwrap().parent().unwrap().to_owned()

@albertlarsan68
Copy link
Member

I corrected quite a few during the move, this one slipped through the cracks of not testing compiletest's output in this case.

Can a test be added ?

@GuillaumeGomez
Copy link
Member Author

That's a good suggestion. Do we have tests for test runners already?

@albertlarsan68
Copy link
Member

albertlarsan68 commented Jan 18, 2023

I don't think so. Should I open an issue ?

EDIT: There is already #47606, which has some discussion and does not have a definite answer.

@tmiasko
Copy link
Contributor

tmiasko commented Jan 18, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Jan 18, 2023

📌 Commit f46f923 has been approved by tmiasko

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 Jan 18, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 19, 2023
…llaumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#105977 (Transform async `ResumeTy` in generator transform)
 - rust-lang#106927 (make `CastError::NeedsDeref` create a `MachineApplicable` suggestion)
 - rust-lang#106931 (document + UI test `E0208` and make its output more user-friendly)
 - rust-lang#107027 (Remove extra removal from test path)
 - rust-lang#107037 (Fix Dominators::rank_partial_cmp to match documentation)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d1c879f into rust-lang:master Jan 19, 2023
@rustbot rustbot added this to the 1.68.0 milestone Jan 19, 2023
@GuillaumeGomez GuillaumeGomez deleted the rm-extra-removal branch January 19, 2023 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants