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

More thorough status-quo tests for #[coverage(..)] #126621

Merged
merged 3 commits into from
Jun 18, 2024

Conversation

Zalathar
Copy link
Contributor

In light of the stabilization push at #84605 (comment), I have written some tests to more thoroughly capture the current behaviour of the #[coverage(..)] attribute.

These tests aim to capture the current behaviour, which is not necessarily the desired behaviour. For example, some of the error message are not great, some things that perhaps ought to cause an error do not, and recursive coverage attributes have not been implemented yet.

@rustbot label +A-code-coverage

@rustbot
Copy link
Collaborator

rustbot commented Jun 18, 2024

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) labels Jun 18, 2024
@petrochenkov
Copy link
Contributor

Could you mark the places in tests that show undesirable or at least questionable behavior with FIXME comments?
@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 Jun 18, 2024
This test reflects the current implementation behaviour, which is not
necessarily the desired behaviour.
…tions

These tests reflect the current implementation behaviour, which is not
necessarily the desired behaviour.
@Zalathar
Copy link
Contributor Author

Added FIXME comments (diff).

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 18, 2024
@petrochenkov
Copy link
Contributor

Thanks!
@bors r+

@bors
Copy link
Contributor

bors commented Jun 18, 2024

📌 Commit 5093658 has been approved by petrochenkov

It is now in the queue for this repository.

@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-review Status: Awaiting review from the assignee but also interested parties. labels Jun 18, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 18, 2024
…llaumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#125988 (Migrate `run-make/used` to `rmake.rs`)
 - rust-lang#126500 (Migrate `error-found-staticlib-instead-crate`, `output-filename-conflicts-with-directory`, `output-filename-overwrites-input`, `native-link-modifier-verbatim-rustc` and `native-link-verbatim-linker` `run-make` tests to `rmake.rs` format)
 - rust-lang#126583 (interpret: better error when we ran out of memory)
 - rust-lang#126587 (coverage: Add debugging flag `-Zcoverage-options=no-mir-spans`)
 - rust-lang#126621 (More thorough status-quo tests for `#[coverage(..)]`)

r? `@ghost`
`@rustbot` modify labels: rollup
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 18, 2024
Rollup merge of rust-lang#126621 - Zalathar:test-coverage-attr, r=petrochenkov

More thorough status-quo tests for `#[coverage(..)]`

In light of the stabilization push at rust-lang#84605 (comment), I have written some tests to more thoroughly capture the current behaviour of the `#[coverage(..)]` attribute.

These tests aim to capture the *current* behaviour, which is not necessarily the desired behaviour. For example, some of the error message are not great, some things that perhaps ought to cause an error do not, and recursive coverage attributes have not been implemented yet.

`@rustbot` label +A-code-coverage
@bors bors merged commit 9f455d3 into rust-lang:master Jun 18, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jun 18, 2024
@Zalathar Zalathar deleted the test-coverage-attr branch June 18, 2024 23:34
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 20, 2024
…illot

More status-quo tests for the `#[coverage(..)]` attribute

Follow-up to rust-lang#126621, after I found even more weird corner-cases in the handling of the coverage attribute.

These tests reveal some inconsistencies that are tracked by rust-lang#126658.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 20, 2024
…illot

More status-quo tests for the `#[coverage(..)]` attribute

Follow-up to rust-lang#126621, after I found even more weird corner-cases in the handling of the coverage attribute.

These tests reveal some inconsistencies that are tracked by rust-lang#126658.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 20, 2024
Rollup merge of rust-lang#126659 - Zalathar:test-coverage-attr, r=cjgillot

More status-quo tests for the `#[coverage(..)]` attribute

Follow-up to rust-lang#126621, after I found even more weird corner-cases in the handling of the coverage attribute.

These tests reveal some inconsistencies that are tracked by rust-lang#126658.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants