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

testsuite: convert unit tests to TAP #121

Merged
merged 15 commits into from
Jan 14, 2025
Merged

testsuite: convert unit tests to TAP #121

merged 15 commits into from
Jan 14, 2025

Conversation

garlick
Copy link
Member

@garlick garlick commented Jan 14, 2025

Problem: the unit tests in tests/misc are run with a very ad hoc test framework and are not co-located with the code they check.

Import libtap (https://github.com/zorgnax/libtap) and rework the existing unit tests to use TAP, cleaning up and relocating them in the process.

Problem: unit testing is very ad-hoc in this project.

Pull in libtap (via the flux-core project), originally from
https://github.com/zorgnax/libtap
Problem: the 9p2000.L protocol encoding test run by tests/misc/t07
presents results in an ad-hoc manner.

Convert to TAP and co-locate with the code under test.
Problem: the fidpool test run by tests/misc/t12 presents results in
an ad hoc manner.

Convert to TAP and co-locate with the code under test.
Problem: the command line opt test run by tests/misc/t05 presents
results in an ad hoc manner.

Convert to TAP and co-locate with the code under test.
Problem: the liblsd/list test run by tests/misc/t09 presents
results in an ad hoc manner.

Convert to TAP and co-locate with the code under test.
Problem: the capability test run by tests/misc/t11 presents
results in an ad hoc manner.

Convert to TAP and co-locate with the code under test.

This test is skipped if it is not run as root.
Add scripts/check-root.sh to run unit tests as root that need it,
aborting if passwordless sudo does not work.
Problem: the fcntl locking test run by tests/misc/t04 presents
results in an ad hoc manner.

Convert to TAP and co-locate with the code under test.
Problem: the setfsuid test run by tests/misc/t00 presents
results in an ad hoc manner.

Convert to TAP and co-locate with the code under test.

This test requires root, so add it to scripts/check-root.sh.
Problem: the setgroups tests run by tests/misc/t01 and t02
present results in an ad hoc manner.

Convert the two ad hoc tests to one TAP test and co-locate
with the code under test.

This test requires root, so add it to scripts/check-root.sh.
Problem: the setreuid test run by tests/misc/t03 presents
results in an ad hoc manner.

Convert to TAP and co-locate with the code under test.

This test requires root, so add it to scripts/check-root.sh.
Problem: the config file tests run by tests/misc/t06 and t08
present results in an ad hoc manner.

Convert to TAP and co-locate with the code under test.
Add this test to scripts/check-valgrind.sh as that might catch problems.

Drop the "tlua" program for checking whether diod was built with
config file support, as this is no longer needed.
Problem: the simple client/server test run by tests/misc/t10 presents
results in an ad hoc manner.

Convert to TAP and co-locate with the code under test.
Add this test to scripts/check-valgrind.sh.

Note: a sleep(1) was dropped from the end of the test and changed the
np_srv_wait_conncount() count from 1 to 0.  I think chaos#98 makes this
possible without memory leaks.
Problem: the directory/file client/server test run by tests/misc/t13
presents results in an ad hoc manner.

Convert to TAP, split into read and directory tests, and co-locate with
the code under test.

Add these tests to scripts/check-valgrind.sh.
Problem: the multiuser client/server test run by tests/misc/t14
presents results in an ad hoc manner.

Convert to TAP, split into read and directory tests, and co-locate with
the code under test.

This test requires root, so add it to scripts/check-root.sh.
Problem: reworked unit tests are never run under valgrind and
some require root.

Run scripts/check-valgrind.sh and scripts/check-root.sh from ci.
Copy link
Member

@grondo grondo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@garlick
Copy link
Member Author

garlick commented Jan 14, 2025

Thanks! I'll set MWP.

@mergify mergify bot merged commit b9a3ee9 into chaos:master Jan 14, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants