junitparser.py: Remove `skipped` attribute from `testsuites` element #101

carlescufi · 2022-11-21T18:16:10Z

The presence of the "skipped" attribute in the "testsuites" element makes validation against the junit/xunit2 schemas fail. Remove it so that xml output is compliant.

See schemas: Jenkins xunit plugin or pytest, as well as the JUnit spec.

Fixes #100.

Signed-off-by: Carles Cufi carles.cufi@nordicsemi.no

carlescufi · 2022-11-21T18:16:21Z

@weiwei FYI

carlescufi · 2022-11-28T09:41:54Z

@EnricoMi now that you are a co-maintainer, would you mind taking a look at this PR? 😃 Thanks!

EnricoMi · 2022-11-29T09:12:38Z

junitparser/junitparser.py

@@ -231,7 +231,6 @@ class JUnitXml(Element):
        tests: total number of tests
        failures: number of failed cases
        errors: number of cases with errors
-        skipped: number of skipped cases


I am not sure about removing this, I'll have to check some specs. At least this has to be made backward-compatible or released under a major version.

I understand. I linked the specs I could find in the commit message / PR description.

EnricoMi · 2022-11-29T09:15:24Z

You need

jobs:
  build-test:
    # Python version < 3.7 requires ubuntu-20.04
    runs-on: ubuntu-20.04

in .github/workflows/build.yml (see #103).

carlescufi · 2022-11-29T09:24:35Z

You need

jobs:
  build-test:
    # Python version < 3.7 requires ubuntu-20.04
    runs-on: ubuntu-20.04

in .github/workflows/build.yml (see #103).

Do you want me to make this change in this PR, or just wait until #103 is merged and then I rebase on top of master?

codecov · 2022-11-29T09:25:28Z

Codecov Report

Base: 98.00% // Head: 98.00% // No change to project coverage 👍

Coverage data is based on head (ffc0f74) compared to base (4326a67).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #101   +/-   ##
=======================================
  Coverage   98.00%   98.00%           
=======================================
  Files           6        6           
  Lines        1253     1253           
=======================================
  Hits         1228     1228           
  Misses         25       25

Impacted Files	Coverage Δ
junitparser/junitparser.py	`99.14% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

EnricoMi · 2022-12-09T08:45:43Z

So from all the specs that I could find it looks the right thing to remove skipped.

Ideally, you would only want it to be removed from the written XML, but still provide the skipped-sum over contained suites, if that is easily possible. Is it?

carlescufi · 2022-12-09T11:16:58Z

So from all the specs that I could find it looks the right thing to remove skipped.

Ideally, you would only want it to be removed from the written XML, but still provide the skipped-sum over contained suites, if that is easily possible. Is it?

That's a good question. I think it might be with some refactoring, by using a class member variable instead of an Attr. I will try to take a look.

carlescufi · 2022-12-11T17:47:25Z

So from all the specs that I could find it looks the right thing to remove skipped.
Ideally, you would only want it to be removed from the written XML, but still provide the skipped-sum over contained suites, if that is easily possible. Is it?

That's a good question. I think it might be with some refactoring, by using a class member variable instead of an Attr. I will try to take a look.

@EnricoMi I just pushed an updated commit that hopefully does what you suggested.

EnricoMi

This way the written XML conforms to the spec while still providing the skipped sum. This reduces risk of breaking user code, but I think it still requires a major version bump.

The presence of the "skipped" attribute in the "testsuites" element makes validation against the junit/xunit2 schemas fail. Remove it so that xml output is compliant. See schemas: https://github.com/jenkinsci/xunit-plugin/blob/master/src/main/resources/org/jenkinsci/plugins/xunit/types/model/xsd/junit-10.xsd#L139-L143 or https://github.com/pytest-dev/pytest/blob/main/testing/example_scripts/junit-10.xsd#L139-L143 as well as: https://www.ibm.com/docs/en/developer-for-zos/14.2?topic=formats-junit-xml-format Fixes weiwei#100. Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>

carlescufi · 2022-12-14T08:54:39Z

@EnricoMi I've rebased the PR now that you've merged #103 so hopefully CI will pass now.

carlescufi · 2022-12-14T09:57:24Z

This way the written XML conforms to the spec while still providing the skipped sum. This reduces risk of breaking user code, but I think it still requires a major version bump.

I agree with you. Is this PR plus #102 enough to justify a major version bump you think?

weiwei · 2022-12-22T02:00:51Z

Sorry I have been busy with family errands this month.

\The schema I referenced was this specific one, because it was the requirement of the project I was working on, and it does need the skipped attribute. I'd like to keep it supported, though I no longer work on that project it anymore.

I've been considering something like xml = JUnitXml(flavor='pytest'). This way we may be able to support different kind of schemas. What do you think? @EnricoMi and @carlescufi

EnricoMi · 2022-12-22T08:59:28Z

Excellent, I like the idea, this allows to support that varying zoo of JUnit XML schemata. And it avoids a breaking change.

carlescufi · 2022-12-22T10:14:16Z

Excellent, I like the idea, this allows to support that varying zoo of JUnit XML schemata. And it avoids a breaking change.

Sure! I don't think I currently have the time to spend on this, but I will try to find some. If you want to go ahead and open another PR instead I will be happy to review.

weiwei · 2023-05-07T02:01:42Z

Implemented in #108

carlescufi force-pushed the xunit2-fix-validate branch from 3db4fb2 to cca4497 Compare November 21, 2022 18:16

carlescufi changed the title ~~junitparser.py: Remove skipped attribute from testsuites~~ junitparser.py: Remove skipped attribute from testsuites element Nov 21, 2022

EnricoMi requested changes Nov 29, 2022

View reviewed changes

carlescufi force-pushed the xunit2-fix-validate branch from cca4497 to 3e618d5 Compare November 29, 2022 09:23

carlescufi requested a review from EnricoMi November 29, 2022 09:24

carlescufi force-pushed the xunit2-fix-validate branch from 3e618d5 to 03d56a7 Compare November 29, 2022 09:27

carlescufi force-pushed the xunit2-fix-validate branch from 03d56a7 to 4bbe3be Compare December 11, 2022 17:46

EnricoMi approved these changes Dec 14, 2022

View reviewed changes

carlescufi force-pushed the xunit2-fix-validate branch from 4bbe3be to ffc0f74 Compare December 14, 2022 08:54

weiwei mentioned this pull request Apr 20, 2023

implement flavors #108

Merged

weiwei closed this May 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

junitparser.py: Remove `skipped` attribute from `testsuites` element #101

junitparser.py: Remove `skipped` attribute from `testsuites` element #101

carlescufi commented Nov 21, 2022 •

edited

Loading

carlescufi commented Nov 21, 2022

carlescufi commented Nov 28, 2022 •

edited

Loading

EnricoMi Nov 29, 2022

carlescufi Nov 29, 2022

EnricoMi commented Nov 29, 2022

carlescufi commented Nov 29, 2022

codecov bot commented Nov 29, 2022 •

edited

Loading

EnricoMi commented Dec 9, 2022

carlescufi commented Dec 9, 2022

carlescufi commented Dec 11, 2022 •

edited

Loading

EnricoMi left a comment

carlescufi commented Dec 14, 2022

carlescufi commented Dec 14, 2022

weiwei commented Dec 22, 2022

EnricoMi commented Dec 22, 2022

carlescufi commented Dec 22, 2022

weiwei commented May 7, 2023

junitparser.py: Remove skipped attribute from testsuites element #101

junitparser.py: Remove skipped attribute from testsuites element #101

Conversation

carlescufi commented Nov 21, 2022 • edited Loading

carlescufi commented Nov 21, 2022

carlescufi commented Nov 28, 2022 • edited Loading

EnricoMi Nov 29, 2022

Choose a reason for hiding this comment

carlescufi Nov 29, 2022

Choose a reason for hiding this comment

EnricoMi commented Nov 29, 2022

carlescufi commented Nov 29, 2022

codecov bot commented Nov 29, 2022 • edited Loading

Codecov Report

EnricoMi commented Dec 9, 2022

carlescufi commented Dec 9, 2022

carlescufi commented Dec 11, 2022 • edited Loading

EnricoMi left a comment

Choose a reason for hiding this comment

carlescufi commented Dec 14, 2022

carlescufi commented Dec 14, 2022

weiwei commented Dec 22, 2022

EnricoMi commented Dec 22, 2022

carlescufi commented Dec 22, 2022

weiwei commented May 7, 2023

junitparser.py: Remove `skipped` attribute from `testsuites` element #101

junitparser.py: Remove `skipped` attribute from `testsuites` element #101

carlescufi commented Nov 21, 2022 •

edited

Loading

carlescufi commented Nov 28, 2022 •

edited

Loading

codecov bot commented Nov 29, 2022 •

edited

Loading

carlescufi commented Dec 11, 2022 •

edited

Loading