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

Dummy PR to test coverage test changes in CI #114917

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Zalathar
Copy link
Contributor

The x86_64-gnu-llvm-15 job that normally runs on PRs doesn't include the profiler runtime, so most of the coverage tests don't get run on PRs.

(That's why #114875 didn't fail until an actual merge was attempted.)

r? @ghost

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Aug 17, 2023
@Zalathar
Copy link
Contributor Author

@rustbot experimental

@Zalathar
Copy link
Contributor Author

@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 17, 2023
@Zalathar
Copy link
Contributor Author

Zalathar commented Aug 17, 2023

@Zalathar
Copy link
Contributor Author

Now testing #114843 in CI on Linux/Windows/macOS, to ensure I don't get any nasty surprises after it's reviewed.

@Zalathar
Copy link
Contributor Author

Success:

@bors
Copy link
Contributor

bors commented Aug 29, 2023

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

@Zalathar Zalathar force-pushed the dummy-coverage branch 2 times, most recently from b605cc7 to c4511d7 Compare September 2, 2023 00:08
@Zalathar
Copy link
Contributor Author

Zalathar commented Sep 2, 2023

Testing #114843 on Linux/Windows/macOS (coverage-map and run-coverage only), to see if everything still works on the big 3 configurations.

@rust-log-analyzer

This comment has been minimized.

@Zalathar
Copy link
Contributor Author

Zalathar commented Sep 2, 2023

Those Linux failures look like the same ones I saw on the aarch64-gnu build, so hopefully that's a platform issue and I don't have to worry about ARM Linux specifically.

I'm going to try another run of Windows only, to flush out any unknown issues on that platform.

@rust-log-analyzer

This comment has been minimized.

@Zalathar
Copy link
Contributor Author

Zalathar commented Sep 2, 2023

Test failures on Windows seem to be the same as the Linux ones, which is surprising.

Perhaps there is some common property of the main Linux/Windows CI environments that is not shared by x86_64-gnu-llvm-15 or my local Aarch64 macOS environment.

@Zalathar
Copy link
Contributor Author

Zalathar commented Sep 2, 2023

I'll try a CI macOS build to double check all the major platforms.

@rust-log-analyzer

This comment has been minimized.

@Zalathar
Copy link
Contributor Author

Zalathar commented Sep 2, 2023

Hmm, so the overall results so far seem to be:

Failure (probably the same failures, though I haven't fully verified this):

  • aarch64-gnu
  • x86_64-gnu
  • x86_64-msvc
  • x86_64-apple-1

Success:

  • x86_64-gnu-llvm-15
  • Local build on aarch64 macOS
  • Local build on aarch64 macOS (--stage=2)

@Zalathar
Copy link
Contributor Author

Zalathar commented Sep 2, 2023

Trying another build with the default PR CI config (x86_64-gnu-llvm-15) just to confirm again that that one still works.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@Zalathar
Copy link
Contributor Author

Testing some details of #119034 in advance.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@Zalathar
Copy link
Contributor Author

As expected, the test does fail on Windows, so it's correct to // ignore-windows.

@bors
Copy link
Contributor

bors commented Jan 5, 2024

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

@Zalathar
Copy link
Contributor Author

Zalathar commented Feb 3, 2024

Testing out a port of tests/run-make/coverage-llvmir to tests/codegen.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Feb 16, 2024

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

@bors
Copy link
Contributor

bors commented Mar 2, 2024

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

@Zalathar
Copy link
Contributor Author

Testing whether I can remove //@ ignore-windows from tests/mir-opt/instrument_coverage.rs.

@Zalathar
Copy link
Contributor Author

Looks like the test passed on Windows: https://github.com/rust-lang/rust/actions/runs/8295637584/job/22703073495?pr=114917

Might as well try mingw as well, since that supposedly has problems with the profiler runtime.

@Zalathar
Copy link
Contributor Author

@rust-log-analyzer

This comment has been minimized.

@Zalathar
Copy link
Contributor Author

Zalathar commented Mar 16, 2024

OK, coverage-run tests are still broken on windows-gnu.

I think the reason tests/mir-opt/instrument_coverage.rs worked is that it doesn't actually run anything, so it doesn't care about having a broken profiler_runtime.

This suggests that maybe we should stop building the profiler runtime on windows-gnu, and compensate by migrating various existing tests from //@ needs-profiler-support to //@ compile-flags: -Zno-profiler-runtime as appropriate.

@bors
Copy link
Contributor

bors commented Mar 18, 2024

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

@bors
Copy link
Contributor

bors commented Apr 15, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants