Skip to content

support branch coverage for testing #24980

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

Merged
merged 6 commits into from
Apr 14, 2025

Conversation

eleanorjboyd
Copy link
Member

fixes #24976

@eleanorjboyd eleanorjboyd added feature-request Request for new features or functionality area-testing labels Apr 11, 2025
@eleanorjboyd eleanorjboyd added this to the April 2025 milestone Apr 11, 2025
@eleanorjboyd eleanorjboyd self-assigned this Apr 11, 2025
@eleanorjboyd
Copy link
Member Author

@danila-grobov heads up on this new testing feature add. Gave it a try and it seemed to show the right results with Django but would love your input if there is anything I got wrong / should correct! I can also reach out again once it is out on insiders and you could test it there if that's easier.

@danila-grobov
Copy link

Hi, @eleanorjboyd

Wow, that was a major pain to set this up to work, I might look into updating the documentation :D

Good job on the feature, it works!

I did spot one weird thing, nothing Django related, but after branch feature introduction the coverage percentages are a bit weird.

image

Before the branch feature it was straight forward it just showed the percentage of statements covered - 6/7 in this case.

Now it shows the percentage of statements and branches covered together 7/9. Which in my opinion doesn't really make sense since branches and statements are just different measurements.

I think we should either leave it as it is, but show the statement percentages instead of the total.

Or we could add another percentage field near branch coverage like we do to the left of the statement coverage the same could be to the left of the branch coverage.

@eleanorjboyd
Copy link
Member Author

Hi! Thanks so much for trying it and sorry it was hard to setup! Which parts were challenging? I will update our wiki accordingly (or docs updates as I know we have to make doc changes to talk about branch coverage and that only coverage > 7.7 version).

Secondly that "total" number is done the same as coveragepy does it if you print to the command line from what I can see (maybe some differences in rounding). Here is what their docs say about it and an example from this test.

    # Name          Stmts   Miss Branch BrPart  Cover
    # -----------------------------------------------
    # mybranch.py       5      0      2      1    86%

so I think we are calculating it the same way as coveragepy for that overall "cover" number but let me know if you are seeing something different. Thanks!

@eleanorjboyd eleanorjboyd marked this pull request as ready for review April 14, 2025 17:01
Copy link

⚠️ This PR originates from a fork. Due to security restrictions, pipelines from forks are no longer triggered automatically. Learn more.

If the changes appear safe, you can manually trigger the pipeline by commenting /AzurePipelines run.

@danila-grobov
Copy link

@eleanorjboyd
Wow, that's a very detailed explanation, thank you! Makes a lot more sense why we would calculate it that way now.

Regarding the issues I faced when setting up things locally:

I've now read the documentation again and it seems it was just my fault. There's a section that states that I have to run "nox --session setup_repo", which would've solved my problems.

In my case, I went through logs and figured out that there's python-env-tools/bin/pet executable missing. And it must be present in order for the extension to work. So I had to go into my extensions folder and copy that executable into my development folder.

I think that maybe this statement threw me off initially:

Do note that building the extension is to discover compiler issues, not to specifically launch the extension in a Development Extension Host window (press F5).

@eleanorjboyd eleanorjboyd merged commit 4b7650e into microsoft:main Apr 14, 2025
46 checks passed
@eleanorjboyd
Copy link
Member Author

@danila-grobov thanks for asking the question as it allowed me to confirm that all our calculations were working right!

ah gotcha yeah the nox is useful but can be confusing. Thanks for the heads up and ill take a look at that specific statement you sent!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-testing feature-request Request for new features or functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Branch Coverage for Testing
3 participants