cli: Simplify junit representation of code owners #39020
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as duplicate.
This comment was marked as duplicate.
fa55486 to
40e0e64
Compare
|
/test |
christarazi
approved these changes
Apr 21, 2025
tklauser
requested changes
Apr 22, 2025
Wrap the codeowners type up so that the rest of the CLI doesn't need to import or use the original library. This will be useful as we extend the type to natively support excluded owners, and it should make it easier to move more codeowners logic into this package to reduce complexity in the other CLI packages. Signed-off-by: Joe Stringer <joe@cilium.io>
Move the code owners exclusions into the codeowners package. Unused for now, but an upcoming commit will move the main logic into this package which will make it simpler to adopt this representation. Signed-off-by: Joe Stringer <joe@cilium.io>
Move the main codeowners logic into the dedicated package. Only minor changes here for function signatures to allow the package to not worry about logging internally but instead return owners errors up to the caller for handling. Signed-off-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Joe Stringer <joe@cilium.io>
Move the detection and encoding of the GitHub owner up into the test_suite in order to (a) de-duplicate this ownership reporting and (b) allow the owner property to be simplified to no longer include the test name / github workflow in a custom format. Signed-off-by: Joe Stringer <joe@cilium.io>
There's no need to include the scenario name in the owners field of the junit, since the parent test is already named in the parent node. Drop it. However, this _is_ still useful context to keep in the text output to help inform developers consulting the test logs about which test / workflow failed or which package emitted an error log. Keep it there. Signed-off-by: Joe Stringer <joe@cilium.io>
40e0e64 to
f6d38ca
Compare
|
/test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The first half of this PR is just tidying up the codeowners logic to move it
into its own package for better reuse elsewhere in the CLI.
The second half reworks the way that codeowners are emitted into the junit
files. Previously, code owners for test cases were emitted multiple times
per test case in the following form, once per owner per scenario and once
per owner of the workflow:
This form will continue to be used for the text output in CI runs, but it will
be changed for the junit format that is reported into the CI dashboard.
Following this PR, the workflow owners are instead injected once at the top
'test_suite' node of the junit, which avoids duplication. These are injected
without the test name, since that is available in the parent node. Furthermore,
rather than duplicating the owners properties for each scenario in a test case,
each owner is included just once.
For an alternative visualization, inspect the following example junit diff,
generated via:
Note that this example only shows the diff for the top of the file; every test case would have the equivalent of the latter hunk applied, dropping the github owners and simplifying the individual owners for the test case.