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

Don't rely on mtime to test changes #13143

Merged
merged 1 commit into from
Dec 8, 2023
Merged

Conversation

ChrisDenton
Copy link
Member

What does this PR try to resolve?

Fixes #13139 by making sure tests aren't relying on changing the mtime alone

How should we test and review this PR?

The pattern to watch out for is when a file is created with empty contents, e.g. .file("foo", ""), then later the same file updated with empty contents, e.g. .change_file("foo", ""). Tests should be making an actual change to the contents.

@rustbot
Copy link
Collaborator

rustbot commented Dec 8, 2023

r? @weihanglo

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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 8, 2023
@weihanglo
Copy link
Member

Seems unrelated, but I wonder what it is 🤔

---- features::no_feature_doesnt_build stdout ----
running `D:\a\cargo\cargo\target\debug\cargo.exe build`
running `D:\a\cargo\cargo\target\tmp\cit\t1334\foo\target\debug\foo.exe`
running `D:\a\cargo\cargo\target\debug\cargo.exe build --features bar -v`
thread 'features::no_feature_doesnt_build' panicked at tests\testsuite\features.rs:525:10:

test failed running `D:\a\cargo\cargo\target\debug\cargo.exe build --features bar -v`
error: process exited with code 101 (expected 0)
--- stdout

--- stderr
   Compiling bar v0.0.1 (D:\a\cargo\cargo\target\tmp\cit\t1334\foo\bar)
     Running `rustc --crate-name bar bar\src\lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C debuginfo=2 -C metadata=66d2c9045771087b -C extra-filename=-66d2c9045771087b --out-dir D:\a\cargo\cargo\target\tmp\cit\t1334\foo\target\debug\deps -L dependency=D:\a\cargo\cargo\target\tmp\cit\t1334\foo\target\debug\deps`
   Compiling foo v0.0.1 (D:\a\cargo\cargo\target\tmp\cit\t1334\foo)
     Running `rustc --crate-name foo src\main.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type bin --emit=dep-info,link -C embed-bitcode=no -C debuginfo=2 --cfg "feature=\"bar\"" -C metadata=7fe94a9cbff53163 -C extra-filename=-7fe94a9cbff53163 --out-dir D:\a\cargo\cargo\target\tmp\cit\t1334\foo\target\debug\deps -L dependency=D:\a\cargo\cargo\target\tmp\cit\t1334\foo\target\debug\deps --extern bar=D:\a\cargo\cargo\target\tmp\cit\t1334\foo\target\debug\deps\libbar-66d2c9045771087b.rlib`
error: couldn't create a temp dir: Access is denied. (os error 5) at path "C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\rustcXRSPPA"

error: could not compile `foo` (bin "foo") due to 1 previous error

Caused by:
  process didn't exit successfully: `rustc --crate-name foo src\main.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type bin --emit=dep-info,link -C embed-bitcode=no -C debuginfo=2 --cfg "feature=\"bar\"" -C metadata=7fe94a9cbff53163 -C extra-filename=-7fe94a9cbff53163 --out-dir D:\a\cargo\cargo\target\tmp\cit\t1334\foo\target\debug\deps -L dependency=D:\a\cargo\cargo\target\tmp\cit\t1334\foo\target\debug\deps --extern bar=D:\a\cargo\cargo\target\tmp\cit\t1334\foo\target\debug\deps\libbar-66d2c9045771087b.rlib` (exit code: 1)

@ChrisDenton
Copy link
Member Author

That seems weird but yeah, I'm not seeing how it could be related to this PR unless tests are tripping over each other (which they shouldn't be, obv). But I'm also not seeing it affecting other PRs. Maybe spurious?

An access denied error can occur for a few different reasons, including permissions errors or if the directory exists and a delete is pending. But I'm not sure any apply to a temp dir.

@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 8, 2023

📌 Commit 4ef503f has been approved by weihanglo

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 Dec 8, 2023
@bors
Copy link
Contributor

bors commented Dec 8, 2023

⌛ Testing commit 4ef503f with merge 047dc18...

@bors
Copy link
Contributor

bors commented Dec 8, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 047dc18 to master...

@bors bors merged commit 047dc18 into rust-lang:master Dec 8, 2023
17 of 20 checks passed
@ChrisDenton ChrisDenton deleted the test-changes branch December 8, 2023 22:18
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 9, 2023
Update cargo

12 commits in 9787229614b27854cf73d57ffae430d7c1e6caa4..6feabf94773286928b7d73d0d313c0c5562b66af
2023-12-06 02:29:23 +0000 to 2023-12-08 22:38:37 +0000
- fix: explicitly remap current dir by using `.` (rust-lang/cargo#13114)
- Don't rely on mtime to test changes (rust-lang/cargo#13143)
- refactor: Pull PackageIdSpec into schema (rust-lang/cargo#13128)
- fix: Print rustc messages colored on wincon (rust-lang/cargo#13140)
- Add a windows manifest file (rust-lang/cargo#13131)
- Avoid writing CACHEDIR.TAG if it already exists (rust-lang/cargo#13132)
- re-enable flaky tests thanks to update to `gix-config`. (rust-lang/cargo#11821) (rust-lang/cargo#13130)
- fix bash completion in directory with spaces (rust-lang/cargo#13126)
- test: re-ignore git auth tests for gitoxide (rust-lang/cargo#13129)
- fix(toml): Disallow inheriting of dependency public status (rust-lang/cargo#13125)
- re-enable previously disabled tests with Windows-specific fix (rust-lang/cargo#13117)
- refactor: Clarify PackageId constructor names (rust-lang/cargo#13123)

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 9, 2023
Update cargo

13 commits in 9787229614b27854cf73d57ffae430d7c1e6caa4..66ad359b408ccdf867cceb30cc2dff1ed4afd82d
2023-12-06 02:29:23 +0000 to 2023-12-09 12:30:01 +0000
- chore: downgrade to openssl v1.1.1 (rust-lang/cargo#13144)
- fix: explicitly remap current dir by using `.` (rust-lang/cargo#13114)
- Don't rely on mtime to test changes (rust-lang/cargo#13143)
- refactor: Pull PackageIdSpec into schema (rust-lang/cargo#13128)
- fix: Print rustc messages colored on wincon (rust-lang/cargo#13140)
- Add a windows manifest file (rust-lang/cargo#13131)
- Avoid writing CACHEDIR.TAG if it already exists (rust-lang/cargo#13132)
- re-enable flaky tests thanks to update to `gix-config`. (rust-lang/cargo#11821) (rust-lang/cargo#13130)
- fix bash completion in directory with spaces (rust-lang/cargo#13126)
- test: re-ignore git auth tests for gitoxide (rust-lang/cargo#13129)
- fix(toml): Disallow inheriting of dependency public status (rust-lang/cargo#13125)
- re-enable previously disabled tests with Windows-specific fix (rust-lang/cargo#13117)
- refactor: Clarify PackageId constructor names (rust-lang/cargo#13123)
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 12, 2023
Update cargo

20 commits in 9787229614b27854cf73d57ffae430d7c1e6caa4..1aa9df1a5be205cce621f0bc0ea6062a5e22a98c
2023-12-06 02:29:23 +0000 to 2023-12-12 14:52:31 +0000
- crates-io: Add support for other 2xx HTTP status codes (rust-lang/cargo#13158)
- Remove the deleted feature test_2018_feature from the test (rust-lang/cargo#13156)
- refactor(schema): Remove reliance on cargo types (rust-lang/cargo#13154)
- fix(toml)!: Disallow `[lints]` in virtual workspaces (rust-lang/cargo#13155)
- Limit exported-private-dependencies lints to libraries (rust-lang/cargo#13135)
- chore: update to gix-index@0.27.1 (rust-lang/cargo#13148)
- Update curl-sys to bring in curl 8.5.0 (rust-lang/cargo#13147)
- chore: downgrade to openssl v1.1.1 (rust-lang/cargo#13144)
- fix: explicitly remap current dir by using `.` (rust-lang/cargo#13114)
- Don't rely on mtime to test changes (rust-lang/cargo#13143)
- refactor: Pull PackageIdSpec into schema (rust-lang/cargo#13128)
- fix: Print rustc messages colored on wincon (rust-lang/cargo#13140)
- Add a windows manifest file (rust-lang/cargo#13131)
- Avoid writing CACHEDIR.TAG if it already exists (rust-lang/cargo#13132)
- re-enable flaky tests thanks to update to `gix-config`. (rust-lang/cargo#11821) (rust-lang/cargo#13130)
- fix bash completion in directory with spaces (rust-lang/cargo#13126)
- test: re-ignore git auth tests for gitoxide (rust-lang/cargo#13129)
- fix(toml): Disallow inheriting of dependency public status (rust-lang/cargo#13125)
- re-enable previously disabled tests with Windows-specific fix (rust-lang/cargo#13117)
- refactor: Clarify PackageId constructor names (rust-lang/cargo#13123)
@ehuss ehuss added this to the 1.76.0 milestone Mar 2, 2024
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

testsuite: explictly update the mtime when needed
5 participants