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

LLVM Coverage and "assert_eq!" #82853

Closed
scole66 opened this issue Mar 7, 2021 · 7 comments
Closed

LLVM Coverage and "assert_eq!" #82853

scole66 opened this issue Mar 7, 2021 · 7 comments
Labels
A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) C-bug Category: This is a bug.

Comments

@scole66
Copy link

scole66 commented Mar 7, 2021

When using -Zinstrument-coverage, my coverage reports often (but not always), report things like:

res::strings::tests::iterator_test_01:
  253|      1|    fn iterator_test_01() {
  254|      1|        let mystr = "blue\u{E00E}";
  255|      1|        let i = CharToU16Iterator::new(mystr);
  256|      1|        let res: Vec<u16> = i.collect();
  257|      0|        assert_eq!(res, &[98, 108, 117, 101, 0xe00e]);
  258|      1|    }

I.e.: the assert_eq! is marked with no coverage. (Now, it's a whole other can of worms for why my testcases themselves should be included in coverage, but I think that's a question for the llvm people, not the rust people.)

Naturally, my expectation is that all the lines of this non-branching testcase have the same execution counts.

I've changed things around, on occasion, to assert!(a == b) instead, and that seems to resolve things, but I'd really rather not have to go to that step.

Meta

rustc --version --verbose:

rustc 1.52.0-nightly (caca2121f 2021-03-05)
binary: rustc
commit-hash: caca2121ffe4cb47d8ea2d9469c493995f57e0b5
commit-date: 2021-03-05
host: x86_64-unknown-linux-gnu
release: 1.52.0-nightly
LLVM version: 12.0.0
@scole66 scole66 added the C-bug Category: This is a bug. label Mar 7, 2021
@wesleywiser wesleywiser added the A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) label Mar 8, 2021
@richkadel
Copy link
Contributor

@scole66 PR #84582 was merged this morning. I believe the change also resolves this issue.

The commits from that PR should be available in the next nightly (tonight or tomorrow).

If possible, can you verify this next week and update/close this issue?

Thanks!

@scole66
Copy link
Author

scole66 commented May 2, 2021

I've been eagerly typing rustup update all week long. ;) As soon as it's in the nightly channel, I'll give it a look.

@scole66
Copy link
Author

scole66 commented May 3, 2021

... hits the nightly channel along with rls and rustfmt, anyway...

info: latest update on 2021-05-03, rust version 1.54.0-nightly (6b5de7aae 2021-05-02)
info: skipping nightly which is missing installed components 'rls', 'rustfmt'

@richkadel
Copy link
Contributor

rustup update --force is what I've used to bypass this.

@scole66
Copy link
Author

scole66 commented May 4, 2021

Nice. Didn't know about that. Seems like matches! needs your marker:

  414|      1|        assert!(matches!(&*node, AssignmentExpression::OpAssignment(..)));
                              ^0

But it does seem like all the assert_eq! are fixed now.

@richkadel
Copy link
Contributor

Awesome (mostly 😉 )

Please feel free to update the issue title for the specific issue with matches! or close this and open a new one.

Thanks so much for the feedback!

@scole66
Copy link
Author

scole66 commented May 4, 2021

Ticket for new coverage issue: #84892.

This issue (i.e. assert_eq!()) is done.

@scole66 scole66 closed this as completed May 4, 2021
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) C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

3 participants