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

Design proposal for test coverage #22827

Open
eleanorjboyd opened this issue Feb 1, 2024 · 1 comment
Open

Design proposal for test coverage #22827

eleanorjboyd opened this issue Feb 1, 2024 · 1 comment
Assignees
Labels
area-testing needs proposal Need to make some design decisions

Comments

@eleanorjboyd
Copy link
Member

With the creation of the coverage API in vscode core, it is now possible to enable coverage for the python extension specifically. Below is the proposed design for how test coverage will work and includes some outstanding questions. Comments welcome as we are still in the design stage.

UX

Users can configure their coverage arguments in a similar way to how all run/debug/discovery args for testing. With the redesign of how testing arguments work with the extension of custom arguments this will be the entry point for coverage arguments. There is a new TestRunProfile kind of type coverage which will allow users to configure coverage specific configs that we will use to run coverage.

Secondly the entrypoint to run will be the buttons and other surfaces implemented in core and the default coverage config will be used unless a different one selected.

Implementation:

To implement coverage, the coverage.py library will be used. Our proposal is to not ship coverage.py and instead take the same approach as with pytest where the user must install coverage.py in their environment to use. This means that to utalize, a user will need to 1- install coverage.py, 2- create a coverage config, 3- select to run coverage.

Engineering design:

User coverage args will be sent to the python subproccess in the same means as any test args
if coverage.py is installed, and the coverage args are provided, then coverage will be calculated as described below
using the coverage.py API the python subprocess would then build a coverage payload
the payload design would include:

This would mean that coverage would support line based coverage (statement is the finite unit for coverage by vscode but python's smallest unit is line)
Due to the fact that branch coverage only lists the covered branch, and not the branches missing coverage, branch coverage will not be included at this time. If total branches can be calculated this can be reconsidered as an additional coverage stat provided.
The coverage payload will be read by the extension and displayed to the users

Other Design Concept:

The following idea was also considered: instead of using payloads for communication instead the team considered using the coverage file, like .coverage converted to XML and then read by the extension to parse the coverage and display. The pro of this would be the increase in separation between coverage and the base testing implementation that might make it easier to break out the coverage work into its own extension in the future. The con to the idea is that it will require the testing subprocess to send via information over a socket/pipe and a file thus duplicating the means of communication. It would also mean that the extension would do the heavy lifting in regards to taking the XML coverage data and converting it into the correct information design instead of doing this work on the python side. This fact would be contrary to our goal of increasing our percentage of python in the extension. ultimately we are leaning towards the first design outlined but are still open to discussion.

Given the proposal the following questions still exist:

  • What happens if a person has coverage.py installed, has a config of type coverage but does not include any argument to enable coverage in this config? It is required to put "" if you want to use coverage (to turn it on) but this could be confusing for users since they selected a coverage run so they might expect it to work without the adding arguments step. One option here is to just check that all coverage runs have the right args to run coverage before spinning up the subprocess.
@amankhandelia
Copy link

Do we have a timeline for implementation of the same, or is the design still under discussion and not firm enough to be on roadmap?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-testing needs proposal Need to make some design decisions
Projects
None yet
Development

No branches or pull requests

2 participants