Skip to content

Conversation

czzrr
Copy link
Contributor

@czzrr czzrr commented Aug 20, 2022

As discussed in #79334, when calling e.g.

python x.py test src/test/ui/expr/compound-assignment/eval-order.rs

on Windows, compiletest passes the filter expr/compound-assignment/eval-order.rs to libtest, which instead should be expr\compound-assignment\eval-order.rs, as that is the file found when collecting tests. This is what I fixed.

I'm not sure how to organize a test for this. Any suggestions?

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Aug 20, 2022
@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 20, 2022
@Mark-Simulacrum
Copy link
Member

I don't think we need a test here, so this seems okay.

That said, I am curious why compiletest collection and the path produced by bootstrap differ - maybe we should be trying to fix that?

@czzrr
Copy link
Contributor Author

czzrr commented Aug 20, 2022

By "path produced by bootstrap", do you in this case mean expr/compound-assignment/eval-order.rs?
If so, that's taken directly from the argument.
Calling python x.py test src/test/ui/expr\compound-assignment/eval-order.rs gives expr\compound-assignment/eval-order.rs etc.

@rust-log-analyzer

This comment has been minimized.

@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented Aug 20, 2022

Yeah, I guess this is a symptom of our "magic" test-args generation biting us; the paths under the hood are just represented as Strings, which don't normalize the way Path/PathBuf would in this case on equality.

r=me with commits squashed

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 21, 2022
@czzrr
Copy link
Contributor Author

czzrr commented Aug 21, 2022

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 21, 2022
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 23, 2022

📌 Commit 8998024 has been approved by Mark-Simulacrum

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 Aug 23, 2022
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Aug 23, 2022
Fix wrong compiletest filters on Windows

As discussed in [rust-lang#79334](rust-lang#79334), when calling e.g.
```
python x.py test src/test/ui/expr/compound-assignment/eval-order.rs
```
on Windows, compiletest passes the filter `expr/compound-assignment/eval-order.rs` to libtest, which instead should be `expr\compound-assignment\eval-order.rs`, as that is the file found when collecting tests. This is what I fixed.

I'm not sure how to organize a test for this. Any suggestions?
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Aug 24, 2022
Fix wrong compiletest filters on Windows

As discussed in [rust-lang#79334](rust-lang#79334), when calling e.g.
```
python x.py test src/test/ui/expr/compound-assignment/eval-order.rs
```
on Windows, compiletest passes the filter `expr/compound-assignment/eval-order.rs` to libtest, which instead should be `expr\compound-assignment\eval-order.rs`, as that is the file found when collecting tests. This is what I fixed.

I'm not sure how to organize a test for this. Any suggestions?
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 24, 2022
Fix wrong compiletest filters on Windows

As discussed in [rust-lang#79334](rust-lang#79334), when calling e.g.
```
python x.py test src/test/ui/expr/compound-assignment/eval-order.rs
```
on Windows, compiletest passes the filter `expr/compound-assignment/eval-order.rs` to libtest, which instead should be `expr\compound-assignment\eval-order.rs`, as that is the file found when collecting tests. This is what I fixed.

I'm not sure how to organize a test for this. Any suggestions?
@matthiaskrgr
Copy link
Member

@bors rollup=iffy #100947 (comment)
(this is just a shot into the dark, feel free to roll it back)

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 24, 2022
@klensy
Copy link
Contributor

klensy commented Aug 24, 2022

rustbot should really write something if r+ PR was changed afterwards.

@rust-log-analyzer

This comment has been minimized.

@czzrr
Copy link
Contributor Author

czzrr commented Aug 24, 2022

That was a totally unrelated commit, my bad...
@rustbot ready

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 24, 2022
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup

I don't think this PR was the cause of the hang -- I think that's just a more general issue we're seeing in CI (as yet undiagnosed).

@bors
Copy link
Collaborator

bors commented Aug 27, 2022

📌 Commit 8998024 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 27, 2022
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Aug 27, 2022
Fix wrong compiletest filters on Windows

As discussed in [rust-lang#79334](rust-lang#79334), when calling e.g.
```
python x.py test src/test/ui/expr/compound-assignment/eval-order.rs
```
on Windows, compiletest passes the filter `expr/compound-assignment/eval-order.rs` to libtest, which instead should be `expr\compound-assignment\eval-order.rs`, as that is the file found when collecting tests. This is what I fixed.

I'm not sure how to organize a test for this. Any suggestions?
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 27, 2022
Rollup of 11 pull requests

Successful merges:

 - rust-lang#96240 (Stabilize `const_ptr_offset_from`.)
 - rust-lang#99784 (Make forward compatibility lint deprecated_cfg_attr_crate_type_name deny by default)
 - rust-lang#100811 (Fix wrong compiletest filters on Windows)
 - rust-lang#100924 (Smaller improvements of tidy and the unicode generator)
 - rust-lang#100953 (Update documentation for `write!` and `writeln!`)
 - rust-lang#101018 (rustdoc: omit start/end tags for empty item description blocks)
 - rust-lang#101044 (rustdoc: remove unused CSS for `hidden-by-*-hider`)
 - rust-lang#101046 (rustdoc: remove incorrect CSS selector `.impl-items table td`)
 - rust-lang#101057 (Merge implementations of HIR fn_decl and fn_sig.)
 - rust-lang#101062 (rustdoc: remove empty extern_crates and type="text/javascript" on script)
 - rust-lang#101063 (Merge duplicated CSS rules)

Failed merges:

 - rust-lang#101055 (Use smaller span for suggestions)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7214f0c into rust-lang:master Aug 27, 2022
@cuviper cuviper added this to the 1.65.0 milestone Nov 16, 2023
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-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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.

9 participants