refactor(reporting): handle case where classname is equal to test_case_name #824
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the JUnit processing logic to prevent duplicate test names when the classname equals the test case name. This addresses cases like Cypress component failures where both values are identical.
Key Changes:
- Added logic to check if classname matches test_case_name before concatenating them
- Modified separator from "." to ". " (dot-space) for better readability
- Extracted test_case_name into a variable for reuse
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🔴 👀 Review RequirementsThis rule is failing.
🔴 🔎 ReviewsThis rule is failing.
🟢 🤖 Continuous IntegrationWonderful, this rule succeeded.
🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
🧪 CI InsightsHere's what we observed from your CI run for 5fb8bf9. 🟢 All jobs passed!But CI Insights is watching 👀 |
sileht
left a comment
sileht
left a comment
There was a problem hiding this comment.
I don't think it's a good move.
If the tool that creates a Junit file with the same classname and test_name, we should just keep it as-is. You can't know if that was on purpose or not.
If the reporting is not good due to this it's not a big deal.
As example, this is totally legit and should be reported as TestFooBar::TestFooBar when the code looks like: |
indeed, I agree, then we should just close this ticket I guess The issue here was that we hit a specific error generated on cypress components, which led to this case, I think it's proper to how cypress works and we can't do anything more upstream to fix this Let's close the ticket, agreed ? |
Yes |
…ned list We needed a more spaced reporting for the test lists in order to prevent confusion. We now report the class and test name separately for better clarity. Fixes MRGFY-5937
| for qt_span in quarantined_tests_spans: | ||
| click.echo(f" · {qt_span.name}") | ||
| if qt_span.attributes is not None: | ||
| generate_test_report( |
There was a problem hiding this comment.
You should pass directly qt_span.attributes and handle the if None and cast there to avoid repeating the code.
|
We're not going into that direction as it means the output would be too much different from what we report in the frontend. |
3 participants
In some cases, the classname can be equal to the test_case_name, which leads us to create a test_name with double identical values concatenated. To avoid this, we introduced a new clause.
Fixes MRGFY-5937