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.

Sign up for GitHub

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

Jump to bottom

embunit / unittests: several improvements #16145

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

embunit / unittests: several improvements #16145

wants to merge 9 commits into from

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Mar 4, 2021

Contribution description

While implementing some embUnit tests, I noticed several issues:

  1. OUTPUT=XML is only supported by tests/unittests,
  2. run_check_unittests() is not used by all tests that use embUnit,
  3. run_check_unittests() is not able to recognize output formats other than the default one an OUTPUT=TEXT.
  4. The color outputters do not reset the color properly.

This PR aims to improve things on all of these fronts. The only output format I did not address was OUTPUT=COMPILER (which only prints when a test fails and only the failed assertions). For that, I think it would make most sense to reverse the logic of run_check_unittests(): expect on errors, and use TIMEOUT as the success condition. But that might not desirable for other tests. So I ignored that output format for now.

Testing procedure

The tests changed should still run and succeed, using OUTPUT=XML provides an XML output.

Issues/PRs references

None

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: tests Area: tests and testing framework Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR labels Mar 4, 2021
@miri64 miri64 changed the title Embunit/enh/improvements embunit / unittests: several improvements Mar 4, 2021
@miri64 miri64 added this to the Release 2021.04 milestone Mar 4, 2021
@miri64
Copy link
Member Author

miri64 commented Mar 5, 2021

While implementing some embUnit tests, […]

For the curious ones: it were the tests in #16156 ;-)

@MrKevinWeiss MrKevinWeiss self-requested a review March 5, 2021 16:29
@MrKevinWeiss MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021
@benpicco benpicco added CI: run tests If set, CI server will run tests on hardware for the labeled PR CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 21, 2021
@benpicco
Copy link
Contributor

benpicco commented Sep 2, 2021

Murdock does not like it.

@miri64
Copy link
Member Author

miri64 commented Sep 3, 2021

Murdock does not like it.

Let's give it another run. Looks more like a murdock error than a build error.

@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 3, 2021
@miri64
Copy link
Member Author

miri64 commented Sep 3, 2021

The tests changed should still run and succeed, using OUTPUT=XML provides an XML output.

This still needs to be run manually though ;-)

@miri64 miri64 removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 3, 2021
@miri64
Copy link
Member Author

miri64 commented Sep 3, 2021

See #16803 for why I removed the CI: ready for build label.

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 3, 2021
@miri64 miri64 force-pushed the embunit/enh/improvements branch from 583e5e3 to d4c624e Compare September 3, 2021 13:23
@github-actions github-actions bot added the Area: sys Area: System label Sep 3, 2021
@miri64
Copy link
Member Author

miri64 commented Sep 3, 2021

The OUTPUT definition of tests/gnrc_lorawan was wrong in master (something that would have been spotted if the PR was merged earlier ;-)). Not sure this was the original cause (the output.html of Murdock was still broken), so let's give Murdock another spin.

@miri64
Copy link
Member Author

miri64 commented Sep 3, 2021

(to fix the error in master I had to rebase this PR)

@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 9, 2021
@benpicco
Copy link
Contributor

benpicco commented Sep 9, 2021

Murdock still does not like it.

@miri64
Copy link
Member Author

miri64 commented Sep 9, 2021

Might be the same problem. Will look into it later!

@miri64
Copy link
Member Author

miri64 commented Sep 9, 2021

Might be the same problem. Will look into it later!

Done. Was indeed a wrong OUTPUT definition again :-). Not sure about the suit_manifest test. Seems unrelated.

@miri64
Copy link
Member Author

miri64 commented Sep 9, 2021

(but can reproduce locally)

@miri64
Copy link
Member Author

miri64 commented Sep 9, 2021

Arghs, used the wrong function in that test. Is now fixed.

@miri64 miri64 force-pushed the embunit/enh/improvements branch from 99713f2 to e682a64 Compare September 9, 2021 16:01
@miri64
Copy link
Member Author

miri64 commented Sep 13, 2021

Of course, it was only a matter of time that the somewhat generally named OUTPUT macro will collide at some point, somewhere. Will look into this later. If any one wants to pick up this work in the meantime, I would not mind :-).

@miri64 miri64 added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Sep 13, 2021
@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Apr 16, 2022
@miri64 miri64 removed the State: stale State: The issue / PR has no activity for >185 days label Apr 16, 2022
@benpicco
Copy link
Contributor

Still WIP?

@miri64
Copy link
Member Author

miri64 commented Apr 16, 2022

Still WIP?

Nothing happened (except for Stale-Bot actions) since I set that label, so yes ;-).

@stale
Copy link

stale bot commented Nov 2, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Nov 2, 2022
@maribu maribu added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 2, 2022
@MrKevinWeiss
Copy link
Contributor

Maybe @Teufelchen1 would like to take a look at this?

@stale stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Nov 2, 2022
@miri64 miri64 added the State: waiting for author State: Action by the author of the PR is required label Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

@smlng smlng Awaiting requested review from smlng

@MrKevinWeiss MrKevinWeiss Awaiting requested review from MrKevinWeiss

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR State: waiting for author State: Action by the author of the PR is required State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@miri64 @benpicco @MrKevinWeiss @maribu @kaspar030