Stabilize #[coverage] attribute #130766
Open
Stabilize #[coverage] attribute #130766
+373
−519
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @nnethercote rustbot has assigned @nnethercote. Use |
rustbot
added
S-waiting-on-review
|
Some changes occurred in coverage tests. cc @Zalathar rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead. cc @rust-lang/rust-analyzer |
clarfonthey
force-pushed
the
stable-coverage-attribute
branch
from
dc49513 to
4b64d0d
Compare
This comment has been minimized.
This comment has been minimized.
clarfonthey
force-pushed
the
stable-coverage-attribute
branch
from
4b64d0d to
2a11c1d
Compare
This comment has been minimized.
This comment has been minimized.
clarfonthey
force-pushed
the
stable-coverage-attribute
branch
from
2a11c1d to
8037f0c
Compare
This comment has been minimized.
This comment has been minimized.
|
I will forward this to a more appropriate reviewer: r? @Zalathar |
|
FWIW, I'm currently working on fixing the test errors-- my computer kept crashing which made it difficult to run all the tests. Unfortunately a lot of coverage tests are now off-by-one due to the removal of the line enabling the feature, and they're all going to have to be re-blessed. |
clarfonthey
force-pushed
the
stable-coverage-attribute
branch
from
8037f0c to
53c467b
Compare
This comment has been minimized.
This comment has been minimized.
|
Appear to be getting very weird crashes with the debuginfo tests. From the backtrace: BacktraceI'm just going to run Yeah, okay. It might also be a difference in Python 3.12 that broke something too, but either way, the debuginfo tests show up as failing on my machine and the coverage tests show up as passing also, so, something weird is afoot. |
|
cc @rust-lang/lang |
I'm not exactly sure what debuginfo tests want, but bootstrap and other parts of compiletest AFAIK assumes Python 3.10 exactly, not 3.9 or 3.12. |
That feels worth documenting somewhere, considering how on my end that just shows up as LLVM hard-crashing and not as a wrong Python version. Like, maybe even worth having a virtualenv setup for the repo just to guarantee the right Python version is being used. |
|
I'm not sure if that's the root cause of your crashes, but if it is, then yes |
|
Well, I'm currently installing 3.10, so, we'll hopefully find out! |
|
Actually sorry minor correction: it's not bootstrap that expects Python 3.10 (as I see there is a toolstate check that goes through 3.10+ as well), but something in compiletest (maybe debuginfo tests?) expect Python 3.10 dll to be available. EDIT: I remembered what it is, it's lldb debuginfo tests, that requires Python 3.10 dll to be in the |
|
|
You linked #128392 twice, which I assume is a mistake. |
|
Yeah, I'm giving up on getting the debuginfo tests working. But I'll poke around to see if I can get the other tests to replicate their issues. I would have to downgrade my system Python version, since the tests explicitly link |
|
It seems that even running the test suite via docker, I can't replicate the latest errors. Running Looks like I've stumbled my way into the realm of cursed compiler testing issues that I don't know how to fix. |
3 tasks
wesleywiser
approved these changes
Dec 13, 2024
There was a problem hiding this comment.
Thanks for the PR and the stabilization report @clarfonthey!
I'm satisfied with the implementation and covering tests.
r=me when rebased
clarfonthey
force-pushed
the
stable-coverage-attribute
branch
from
5d6aae4 to
9e17904
Compare
|
Rebased, let's see if I caught everything… |
|
(I would wait for the PR runs to finish first, since last time I had a bunch of issues that weren't caught by my local testing, and I want to verify those pass before we clog up the merge queue.) (Also, there's still those debuginfo tests that don't work without Docker because LLVM doesn't understand what compatibility means. 😠) |
|
Seems to pass! |
|
@bors r=@wesleywiser (I expect some coverage tests to fail) |
bors
added
S-waiting-on-bors
|
@bors p=1 (conflict-prone) |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Dec 16, 2024
…e, r=wesleywiser Stabilize #[coverage] attribute Closes rust-lang#84605, which passed FCP. Stabilisation report here: rust-lang#84605 (comment) Also added to reference here: rust-lang/reference#1628
This comment has been minimized.
This comment has been minimized.
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
(Unrelated to the above failure) Optional suggestion: Instead of deleting (Feel free to only do this if you think it would actually help.) |
clarfonthey
force-pushed
the
stable-coverage-attribute
branch
from
9e17904 to
cccbaae
Compare
|
I think I figured out the issue— one test required the profiler runtime which wasn't tested on the PR tests, but is tested in the merge tests. It should work now. |
|
Giving this a try job on the main targets that run coverage tests: @bors try |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Dec 16, 2024
…e, r=<try> Stabilize #[coverage] attribute Closes rust-lang#84605, which passed FCP. Stabilisation report here: rust-lang#84605 (comment) Also added to reference here: rust-lang/reference#1628 --- try-job: aarch64-apple try-job: x86_64-gnu try-job: x86_64-msvc
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-author
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #84605, which passed FCP.
Stabilisation report here: #84605 (comment)
Also added to reference here: rust-lang/reference#1628
try-job: aarch64-apple
try-job: x86_64-gnu
try-job: x86_64-msvc