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

ci: turn on tcp feature in examples #119

Merged
merged 5 commits into from
Nov 30, 2023
Merged

Conversation

lwshang
Copy link
Contributor

@lwshang lwshang commented Nov 28, 2023

When I was testing the x86_64 example with tcp feature, I got a compilation error at the line of dev.transmit(tx_buf).unwrap();.

The error was introduced in a recent PR: #111.
It was overlooked by CI because the default value tcp=off was in effect.

I found the original rationale for disabling tcp in CI from this commit message: 5b0786b

Disable tcp feature in x86_64 example by default.
It waits forever, which is no good in CI.

So I modify the test_echo_server function to break out the dead loop.

I also applied the same fix to the riscv example.

* the tests can terminate gracefully
* fix a bug in x86_64 tcp module which was not covered in CI
@lwshang lwshang marked this pull request as ready for review November 28, 2023 23:42
@lwshang
Copy link
Contributor Author

lwshang commented Nov 30, 2023

GitHub CI has rustup by default, it can pickup the correct toolchain from rust-toolchain.toml files automatically.

So there is no need to setup toolchain in main.yml anymore. The toml files will be the only source of truth.

examples/aarch64/rust-toolchain.toml Outdated Show resolved Hide resolved
@lwshang lwshang changed the title Tcp on ci: turn on tcp feature in examples Nov 30, 2023
@qwandor
Copy link
Collaborator

qwandor commented Nov 30, 2023

Thanks!

@qwandor qwandor merged commit 501d359 into rcore-os:master Nov 30, 2023
5 checks passed
@lwshang lwshang deleted the tcp_on branch November 30, 2023 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants