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 32bit test suite on PR CI? #69823

Open
RalfJung opened this issue Mar 8, 2020 · 9 comments
Open

Run 32bit test suite on PR CI? #69823

RalfJung opened this issue Mar 8, 2020 · 9 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. P-low Low priority T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

RalfJung commented Mar 8, 2020

@rust-lang/infra do we have enough spare capacity on the PR CI runner to add something like ./x.py test src/test/ui --target i686-unknown-linux-gnu --pass check? That would help find 32bit-only issues in tests before things get to bors. And running with --pass check would also ensure that the test suite actually passes in check-only mode, which is not a given (some tests fail and need ignore-pass).

@RalfJung RalfJung added the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label Mar 8, 2020
@Centril Centril added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Mar 8, 2020
@pietroalbini
Copy link
Member

Nominating for discussion at the infrastructure team meeting.

@Mark-Simulacrum
Copy link
Member

I would be interested in seeing error rates, at least; I have not experienced a lot of pain with 32 bit tests failing (and 64 bit tests passing). OTOH, maybe the --pass check mode is worth it...

Do we have numbers on how long we expect this to take on CI? I would expect someone to run the UI tests that we already run on CI locally and then figure out an approximate "CI slowdown" based on how long those take, to determine how long we'd expect this test to take.

@pietroalbini
Copy link
Member

We discussed this in the infra team meeting, and we'd like to see numbers on the impact this would have in our CI times before making a decision.

@aidanhs aidanhs added P-medium Medium priority and removed I-nominated labels Mar 18, 2020
@aidanhs
Copy link
Member

aidanhs commented Mar 18, 2020

To be more explicit - it's not likely that the infra team will have a chance to proactively look into this.

As stated above, we would be looking for help on understanding:

  • how does this affect our CI times
  • how much of a 'significant bit' is this check (i.e. how often would this catch something that the current check wouldn't)

@pietroalbini
Copy link
Member

Discussed this again in the infrastructure team meeting: we're still waiting for the data before making a decision, and unfortunately nobody in the team has a chance to gather it.

@RalfJung
Copy link
Member Author

RalfJung commented Apr 1, 2020

I am also not sure when I will have the time to do such experiments.

@RalfJung
Copy link
Member Author

RalfJung commented May 8, 2020

@Mark-Simulacrum self-assigned this 2 days ago

Can I take this to mean you plan to look into this? :)

@Mark-Simulacrum
Copy link
Member

Yes, I've been running out of time these past couple days but hopefully will get to it soon.

bors added a commit to rust-lang-ci/rust that referenced this issue Jul 1, 2020
…lbini

Test UI tests for pass=check

I'm going to just compare the builder times since I wasn't able to get this working nicely locally (hit some obscure linker error).

Fixes part of rust-lang#69823
@Mark-Simulacrum Mark-Simulacrum removed their assignment Aug 5, 2020
@Mark-Simulacrum
Copy link
Member

As of #72053, we are running UI tests in check mode on the PR CI builder. We are not yet running them in 32 bit mode, though, because LLVM 8 is broken on cross-compilation (IIRC, don't remember details though).

@pietroalbini pietroalbini added P-low Low priority and removed P-medium Medium priority labels Nov 4, 2020
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 19, 2020
Remove Hacks and Fixmes from PR CI's LLVM-9 Container

Now with LLVM 9 being the minimum supported version (thanks to rust-lang#78848 ), we can
finally remove the hacks in the dockerfile.

This wasn't in the main PR bumping the version as I didn't quite
understand what's going on and needed here.

Relevant issues and PRs:

- Issue rust-lang#69823
- PR rust-lang#70989

I hope I actually adressed things correctly here?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. P-low Low priority T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants