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

boards/native: only use pyterm wrapper with term target #20264

Merged
merged 4 commits into from
Jan 19, 2024

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Jan 16, 2024

Contribution description

Only enable the pyterm wrapper if we are executing the term target to avoid breaking things.

Testing procedure

make debug should work again on native.
So does e.g. tests/sys/log_color without the previously added workaround.

Issues/PRs references

reverts #20215
fixes #20265

@github-actions github-actions bot added Platform: native Platform: This PR/issue effects the native platform Area: tests Area: tests and testing framework Area: build system Area: Build system Area: boards Area: Board ports labels Jan 16, 2024
@benpicco benpicco added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jan 16, 2024
@riot-ci
Copy link

riot-ci commented Jan 16, 2024

Murdock results

✔️ PASSED

e5618f7 boards/native: set and test for pyterm explicitly

Success Failures Total Runtime
8623 0 8623 12m:28s

Artifacts

@github-actions github-actions bot removed Area: tests Area: tests and testing framework Area: build system Area: Build system labels Jan 16, 2024
@github-actions github-actions bot added Area: tests Area: tests and testing framework Area: build system Area: Build system labels Jan 16, 2024
@benpicco benpicco force-pushed the boards/native-term_fix branch 2 times, most recently from 13562ef to 7658e01 Compare January 16, 2024 15:04
@benpicco benpicco added the Community: help wanted The contributors require help from other members of the community label Jan 16, 2024
@OlegHahm
Copy link
Member

I don't understand the fix. Currently nativeis set as RIOT_TERMINAL in various tests (independent of the Make target). So wouldn't this change just additionally set it if it has not been set but still use it for, e.g., make debug if set as is in the tests?

@benpicco
Copy link
Contributor Author

The idea was to only use pyterm on native when the target is term to avoid interfering with other targets (test, debug, valgrind, …) and not having to play whack-a-mole.

That's why this also reverts the change to always use native term for the test target because now it should now always be used except for the term target.

@github-actions github-actions bot added the Area: doc Area: Documentation label Jan 19, 2024
Merged via the queue into RIOT-OS:master with commit 9122949 Jan 19, 2024
25 checks passed
@benpicco benpicco deleted the boards/native-term_fix branch January 19, 2024 17:25
@MrKevinWeiss MrKevinWeiss added this to the Release 2024.01 milestone Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports Area: build system Area: Build system Area: doc Area: Documentation 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 Community: help wanted The contributors require help from other members of the community 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
Development

Successfully merging this pull request may close these issues.

make debug broken on native
5 participants