test: Add test for CycloneDX XML with authors #10271
Conversation
MarcelBochtler
force-pushed
the
invalid-cyclonedx-xml
branch
2 times, most recently
from
f1560eb to
e075508
Compare
| @@ -94,6 +96,30 @@ class CycloneDxReporterFunTest : WordSpec({ | |||
| } | |||
| } | |||
|
|
|||
| "create the expected XML file with authors" { | |||
There was a problem hiding this comment.
I believe instead of adding a new separate test, it would make sense to instead enhance the input of "create the expected XML file". For enhancing it withAuthors() imo is ok. Furthermore, the analog JSON test case IMO should also use / keep on using the same input, so that XML and JSON tests remain analog.
There was a problem hiding this comment.
@MarcelBochtler would you agree with above? (and update the commit)
There was a problem hiding this comment.
@MarcelBochtler would you agree with above? (and update the commit)
I would agree if the tests would pass.
But as the test is currently expected to fail, it would either have to be disabled or not being merged at all.
If I add the withAuthors() to the "create the expected XML file" file, we can no longer disable the test.
I'm not sure how to continue here.
There was a problem hiding this comment.
I'm not sure how to continue here.
I think the best we can do is to either push at CycloneDX/cyclonedx-core-java#638 for a fix, or contribute the fix ourselves.
There was a problem hiding this comment.
I would agree if the tests would pass.
If we re-target this commit to update the test input data to illustrate an issue with the authors (not serialized), wouldn't the be ok? (And once it's fixed upstream and integrated, the expected results would be updated for good). (If this was merged as-is, then we have to revert it and update the existing tests, which seems unnecessary work)
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10271 +/- ##
=========================================
Coverage 56.62% 56.62%
Complexity 1630 1630
=========================================
Files 332 332
Lines 12324 12324
Branches 1139 1139
=========================================
Hits 6979 6979
Misses 4914 4914
Partials 431 431
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
MarcelBochtler
force-pushed
the
invalid-cyclonedx-xml
branch
from
e075508 to
44103f4
Compare
|
@MarcelBochtler how do we proceed here? I believe the test now proves that there is a problem. So are we good to report this bug upstream? |
I reported it upstream already: CycloneDX/cyclonedx-core-java#638 |
👍🏻
Sorry, I was looking the inital post only, which just generally links to https://github.com/CycloneDX/cyclonedx-core-java. |
This test is currently expected to fail due to a bug in the cyclonedx-core-java [1]. [1]: CycloneDX/cyclonedx-core-java#638 Signed-off-by: Marcel Bochtler <[email protected]>
MarcelBochtler
force-pushed
the
invalid-cyclonedx-xml
branch
from
44103f4 to
f71bedb
Compare
This test is currently expected to fail due to a bug in the
cyclonedx-core-java 1.