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

Run standard library unit tests without optimizations in nopt CI jobs #73383

Merged
merged 1 commit into from Aug 14, 2020

Conversation

ecstatic-morse
Copy link
Contributor

@ecstatic-morse ecstatic-morse commented Jun 15, 2020

This was discussed in #73288 as a way to catch similar issues in the future. This builds an unoptimized standard library with the bootstrap compiler and runs the unit tests. This takes about 2 minutes on my laptop.

I confirmed that this method works locally, although there may be a better way of implementing it. It would be better to use the stage 2 compiler instead of the bootstrap one.

Notably, there are currently four libstd unit tests that fail in debug mode on i686-unkown-linux-gnu (a tier one target):

failures:
    f32::tests::test_float_bits_conv
    f32::tests::test_total_cmp
    f64::tests::test_float_bits_conv
    f64::tests::test_total_cmp

These are the tests that prompted #73288 as well as the ones added in #72568, which is currently broken due to #73328.

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

I would rather add a dedicated flag (or perhaps third value for disable-optimize-tests) which builds std unoptimized and tests that; I suspect that should mostly be a matter of adjusting this to gate on rust_optimize_tests if we're and not pass --release if the cmd is "test". (We still want to build a release std).

We'll want to test, but I believe that should just work then on the nopt builders and get us the behavior we want (stage 2 testing).

@Mark-Simulacrum
Copy link
Member

Actually to start off we may want some compiler team survey or so -- maybe there's no need for a separate flag, we can just integrate it directly. I would be fine with that personally but maybe there's value for "compiler" and "std"/test/etc tests being separated out.

@Mark-Simulacrum Mark-Simulacrum 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 Jun 25, 2020
@Mark-Simulacrum
Copy link
Member

I'm going to relabel as waiting-on-author. I'll leave it up to you if you want to bring this up at a meeting (nominating it, I guess) to determine whether a dedicated flag or not makes sense, or instead implement separate flag(s).

@bors
Copy link
Contributor

bors commented Jul 4, 2020

☔ The latest upstream changes (presumably #73650) made this pull request unmergeable. Please resolve the merge conflicts.

@Muirrum
Copy link
Member

Muirrum commented Jul 24, 2020

@ecstatic-morse This is a triage bump

@crlf0710
Copy link
Member

crlf0710 commented Aug 7, 2020

Still needs a rebase...

@crlf0710 crlf0710 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 7, 2020
We already have builders which built standard library *test*s without
optimizations, but we previously did not have builders which built the standard
library itself without optimizations and then tested that.

This adds those builds for i686 and x86_64 linux.
@Mark-Simulacrum
Copy link
Member

@bors try

@bors
Copy link
Contributor

bors commented Aug 13, 2020

⌛ Trying commit b190d29cb370185f56da1952c20422c217d3b2cb with merge b527eeff23cea94181bdef3d291105be5addd1df...

@bors
Copy link
Contributor

bors commented Aug 13, 2020

☀️ Try build successful - checks-actions, checks-azure
Build commit: b527eeff23cea94181bdef3d291105be5addd1df (b527eeff23cea94181bdef3d291105be5addd1df)

@Mark-Simulacrum
Copy link
Member

@bors r+

Okay, I've rebased this and the try build came back successfully, so approving.

Looking back at what I said earlier, I think a dedicated option for "I want to run tests against no-opt versions of std" or so may make sense, but it need not block the minimal version here which doesn't require any bootstrap changes. This PR also gets us stage 0, i.e., cfg(bootstrap) testing of std, which is also a positive.

@bors
Copy link
Contributor

bors commented Aug 13, 2020

📌 Commit 662871f has been approved by Mark-Simulacrum

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 13, 2020
@bors
Copy link
Contributor

bors commented Aug 14, 2020

⌛ Testing commit 662871f with merge 43bec40...

@bors
Copy link
Contributor

bors commented Aug 14, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: Mark-Simulacrum
Pushing 43bec40 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 14, 2020
@bors bors merged commit 43bec40 into rust-lang:master Aug 14, 2020
@ecstatic-morse ecstatic-morse deleted the test-unoptimized-std branch October 6, 2020 01:42
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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.

None yet

7 participants