Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Update Cobertura Output #1728

Merged
merged 53 commits into from
Apr 7, 2022
Merged

Conversation

hayleycall
Copy link
Contributor

@hayleycall hayleycall commented Mar 28, 2022

Summary of the Pull Request

Modified Cobertura output to include line-rates, covered and valid to have the summary tab show up in the Publish Cde Coverage task in the pipeline.

Modified Cobertura output to have file directory as package name and only file name as class name for every file. This creates easier to digest output in pipeline.

PR Checklist

  • Applies to work item: #xxx
  • CLA signed. If not, go over here and sign the CLI.
  • Tests added/passed
  • Requires documentation to be updated
  • I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Info on Pull Request

Modified Cobertura output to include line-rates, covered and valid to have the summary tab show up in the Publish Cde Coverage task in the pipeline.

Modified Cobertura output to have file directory as package name and only file name as class name for every file. This creates easier to digest output in pipeline.

Includes only changes to Cobertura format.

Refactored Cobertura conversion code to be more readable/debuggable and included additional tests.

Validation Steps Performed

Test Updated

src/agent/coverage/src/cobertura.rs Outdated Show resolved Hide resolved
src/agent/coverage/src/cobertura.rs Outdated Show resolved Hide resolved
src/agent/coverage/src/cobertura.rs Outdated Show resolved Hide resolved
src/agent/coverage/src/cobertura.rs Show resolved Hide resolved
src/agent/coverage/src/cobertura.rs Outdated Show resolved Hide resolved
src/agent/coverage/src/cobertura.rs Outdated Show resolved Hide resolved
src/agent/coverage/src/cobertura.rs Outdated Show resolved Hide resolved
src/agent/coverage/src/cobertura.rs Outdated Show resolved Hide resolved
src/agent/coverage/src/cobertura.rs Outdated Show resolved Hide resolved
src/agent/coverage/src/cobertura.rs Outdated Show resolved Hide resolved
src/agent/coverage/src/cobertura.rs Outdated Show resolved Hide resolved
src/agent/coverage/src/cobertura.rs Outdated Show resolved Hide resolved
src/agent/coverage/src/cobertura.rs Outdated Show resolved Hide resolved
Copy link
Member

@ranweiler ranweiler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great!

As discussed offline, the last thing I'd love to see in this: right now, in some tests, we currently "manually" dynamically generate the expected XML in-memory, whenever we run the test, and then compare it to the output of the cobertura() function.

Instead, we can commit hardcoded XML files for the expected output. We should include cases for Windows, POSIX, and mixed paths. Then use include_bytes!()/include_str!() to expose those file contents as &[u8]/&str constants in the tests module. The test assertions can then just compare the cobertura() function's output against those expected XML files, as strings.

This lets us avoid depending on the XML emitter in the test functions, shortens the test bodies, and lets future readers see a full, human-friendly representation of "good output", without any tricks or extra steps.

@hayleycall hayleycall merged commit 3d52790 into microsoft:main Apr 7, 2022
@ghost ghost locked as resolved and limited conversation to collaborators May 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants