Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

count 'skipped' tests as skipped #1121

Merged
merged 1 commit into from
Sep 28, 2020

Conversation

dseifert
Copy link
Contributor

Previously only 'skip' and 'disabled' were counted as skipped tests.
However, junit format explicitly allows 'skipped' (cmp.
https://github.com/windyroad/JUnit-Schema/blob/master/JUnit.xsd#L196)

For reference, also see the same issue in colcon:

This is based on noetic-devel, but applies to kinetic-devel as well (particularly, we are facing this issue under melodic and would appreciate a backport (or shall I create a new PR?))

Previously only 'skip' and 'disabled' were counted as skipped tests.
However, junit format explicitly allows 'skipped' (cmp.
https://github.com/windyroad/JUnit-Schema/blob/master/JUnit.xsd#L196)

For reference, also see the same issue in colcon:
- Bug: colcon/colcon-test-result#18
- PR: colcon/colcon-test-result#19
@dirk-thomas
Copy link
Member

Thanks for the patch. Yes, support for skipped was added to the schema in the recent years.

@dirk-thomas dirk-thomas merged commit c43325c into ros:noetic-devel Sep 28, 2020
dirk-thomas pushed a commit that referenced this pull request Sep 28, 2020
Previously only 'skip' and 'disabled' were counted as skipped tests.
However, junit format explicitly allows 'skipped' (cmp.
https://github.com/windyroad/JUnit-Schema/blob/master/JUnit.xsd#L196)

For reference, also see the same issue in colcon:
- Bug: colcon/colcon-test-result#18
- PR: colcon/colcon-test-result#19
@dirk-thomas
Copy link
Member

Cherry-picked to kinetic-devel in 55dcf35.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants