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

Test suite refactor #25

Merged
merged 18 commits into from Sep 1, 2022
Merged

Test suite refactor #25

merged 18 commits into from Sep 1, 2022

Conversation

MatteoNardi
Copy link
Contributor

This PR is a tentative refactor of the test suite.
Tests are split in a different binary, which can be invoked with sudo by using a xtask.

Fixes #3

@MatteoNardi MatteoNardi force-pushed the test-suite branch 2 times, most recently from 0e01450 to 747cbf8 Compare July 28, 2022 10:33
@MatteoNardi
Copy link
Contributor Author

All the test were ported to the new system. I'll work on documentation and general cleanup, than it will be ready for review.

@MatteoNardi
Copy link
Contributor Author

This PR is now ready for review.

  • I've split the test suite in a different binary. See README.
  • I've added xtasks for every pulsar binary which need to be run as sudo. See README
    • I've made probe a binary to make it more coherent with the others.
  • cargo test doesn't require sudo anymore. I've re-enabled validatron tests.
  • All documentation references in the repository should have been updated.

Some notes:

  • I've decided to get rid of inventory for tests collection as I don't think the added value justified the added complexity. Every module has a test entrypoint which must be listed in the test-suite binary.

@MatteoNardi MatteoNardi changed the title Test suite refactor [WIP] Test suite refactor Jul 29, 2022
@MatteoNardi MatteoNardi marked this pull request as ready for review July 29, 2022 13:15
Cargo.toml Outdated Show resolved Hide resolved
xtask/src/main.rs Outdated Show resolved Hide resolved
xtask/Cargo.toml Show resolved Hide resolved
test-suite/Cargo.toml Show resolved Hide resolved
bpf-common/src/test_runner.rs Show resolved Hide resolved
I noticed trace pipe logging stopped working and I tracked down the
issue to the original file descriptor being closed too soon (when
returning from open_trace_pipe). Turns out AsyncFd is not taking
ownership the the original file and we must keep it around until we're
done reading)
Previously we decided to make `probe` an example because we had only
another binary (`pulsar-exec`) and it made sense to have it as the
default target.

Now we've added move binaries (`xtask`, `test-suite`) and we don't run
binaries directly with `cargo run` because it lacks root privileges.
@MatteoNardi
Copy link
Contributor Author

I've rebased to main and added 2b3e584

Before this change, the panic message was displayed in the middle of the
test lines. libtest-mimic didn't write `FAILURE` because the test stopped.

After this change, the test completes with a FAILURE line and the panic
message is displayed at the bottom of the screen.
@banditopazzo banditopazzo merged commit 84a611b into main Sep 1, 2022
@banditopazzo banditopazzo deleted the test-suite branch September 13, 2022 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Error when running cargo test
2 participants