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

When compiletest'ing src/test/mir-opt, check timestamps. #39717

Merged

Conversation

pnkfelix
Copy link
Member

The tests in src/test/mir-opt embed references to generated files. The names of the generated files embed node id's, which will change depending on the content of the original source.

To guard against comparisons against stale output, check the timestamps of the supposed output against the timestamp of the original source (i.e. any output should be at least as new as the source that was recompiled).

Fix #39690.

@rust-highfive
Copy link
Collaborator

r? @aturon

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

@alexcrichton
Copy link
Member

@pnkfelix due to our vendoring requirements it'll be easiest to avoid the addition of the time crate for now, perhaps these could just be printed in a raw form for now?

@pnkfelix
Copy link
Member Author

perhaps these could just be printed in a raw form for now?

Ugh, seconds from the epoch is so ugly.

I'll just leave the timestamps out of the panic! message and include the underlying representation in a debug! call.

@pnkfelix pnkfelix force-pushed the check-timestamps-in-compiletest-miropt branch from eac3f68 to ca94a64 Compare February 10, 2017 17:24
@pnkfelix pnkfelix force-pushed the check-timestamps-in-compiletest-miropt branch from 8055031 to 67971e4 Compare February 20, 2017 16:00
This version removes prior use of `time` crate, to satisify vendoring requirements.

remove extraneous whitespace change
@pnkfelix pnkfelix force-pushed the check-timestamps-in-compiletest-miropt branch from 67971e4 to 6a78282 Compare February 20, 2017 16:00
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Feb 20, 2017

📌 Commit 6a78282 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Feb 20, 2017

⌛ Testing commit 6a78282 with merge a17e5e2...

bors added a commit that referenced this pull request Feb 20, 2017
…t, r=alexcrichton

When compiletest'ing src/test/mir-opt, check timestamps.

The tests in src/test/mir-opt embed references to generated files. The names of the generated files embed node id's, which will change depending on the content of the original source.

To guard against comparisons against stale output, check the timestamps of the supposed output against the timestamp of the original source (i.e. any output should be at least as new as the source that was recompiled).

Fix #39690.
@bors
Copy link
Contributor

bors commented Feb 20, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing a17e5e2 to master...

@bors bors merged commit 6a78282 into rust-lang:master Feb 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants