Skip to content

tests: use native terminal where necessary #20213

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
Closed
wants to merge 6 commits into from
Closed

tests: use native terminal where necessary #20213

wants to merge 6 commits into from

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Dec 22, 2023 •
edited
Loading

Contribution description

For some reason those only are detected by CI when disabling Kconfig builds (#20211)

Testing procedure

Issues/PRs references

same as #20212

@benpicco benpicco requested a review from OlegHahm December 22, 2023 21:14
@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Dec 22, 2023
@benpicco benpicco added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed Area: tests Area: tests and testing framework labels Dec 22, 2023
@riot-ci
Copy link

riot-ci commented Dec 22, 2023 •
edited
Loading

Murdock results

✔️ PASSED

4d4afed tests/sys/congure_quic: use native terminal

Success Failures Total Runtime
957 0 957 02m:53s

Artifacts

@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Dec 22, 2023
@benpicco benpicco requested a review from miri64 December 22, 2023 21:33
@benpicco benpicco changed the title tests/sys/log_color: use native terminal tests: use native terminal where necessary Dec 22, 2023
OlegHahm
OlegHahm reviewed Dec 22, 2023
View reviewed changes
@OlegHahm
Copy link
Member

To me it would sound sane to run all native tests without pyterm:

diff --git a/tests/Makefile.tests_common b/tests/Makefile.tests_common
index 5e988d903f..d96751c3ad 100644
--- a/tests/Makefile.tests_common
+++ b/tests/Makefile.tests_common
@@ -17,3 +17,7 @@ RIOTBASE ?= $(CURDIR)/../..
 QUIET ?= 1
 # DEVELHELP enabled by default for all tests, set 0 to disable
 DEVELHELP ?= 1
+
+ifeq (native, $(BOARD))
+  RIOT_TERMINAL ?= native
+endif

@benpicco benpicco force-pushed the pyterm_native-fixes branch from a4842ae to 4d4afed Compare December 23, 2023 16:23
@github-actions github-actions bot added Platform: native Platform: This PR/issue effects the native platform Area: boards Area: Board ports labels Dec 23, 2023
@benpicco benpicco mentioned this pull request Dec 23, 2023
Merged
@benpicco
Copy link
Contributor Author

To me it would sound sane to run all native tests without pyterm

I agree, but we have many 'interactive' tests or tests that are more like examples where having pyterm is nice.

How about only disabling pyterm when executing the test target - #20215

@miri64
Copy link
Member

miri64 commented Jan 2, 2024

I think the release tests need this as well: https://github.com/RIOT-OS/RIOT/actions/runs/7361658214/job/20039229745. :-/

@benpicco
Copy link
Contributor Author

benpicco commented Jan 2, 2024

Closing this in favor of #20215

@benpicco benpicco closed this Jan 2, 2024
@benpicco benpicco deleted the pyterm_native-fixes branch January 2, 2024 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OlegHahm OlegHahm OlegHahm left review comments

@smlng smlng Awaiting requested review from smlng

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

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

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

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

@miri64 miri64 Awaiting requested review from miri64

Assignees
No one assigned
Labels
Area: boards Area: Board ports 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 Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Platform: native Platform: This PR/issue effects the native platform Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@benpicco @riot-ci @OlegHahm @miri64