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: relocate libflux unit tests #1827

Merged
merged 8 commits into from Nov 13, 2018

Conversation

Projects
None yet
3 participants
@garlick
Copy link
Member

garlick commented Nov 10, 2018

This moves some unit tests that were located in the sharness area to libflux/test so that make check in that directory runs all the unit tests that cover the libflux code, as complained about in #1823.

This was difficult before because the tests depended on the connector directory being built. However, it turns out it was pretty simple to build up the "test server" implementation from t/rpc/util.c to be standalone, and then to add a standalone loopback connector so the loop:// tests could be ported. I left the loop:// connector alone since it is used by other tests scattered around the source tree.

The duplication of code between the loop:// and shmem:// connectors and this modified libflux/util.c is not desirable, but IMHO it's not that much code, and the benefits seem worth it.

I tried not to change code and move it in the same commit for reviewability, although in two cases a move was combined with a change to "absorb" a trivial unit test of the same name. Some further (minor) changes were required to make the tests work without the connector directory pre-built, which I kept in a separate commit. That will break git bisect but I thought probably reviewability was more important. They could be squashed if that seems better.

The tests that were relocated are

t/rpc/mrpc.c # absorbed tiny mrpc.c test
t/rpc/rpc.c
t/rpc/rpc_chained.c
t/rpc/dispatch.c
t/rpc/handle.c  # absorbed tiny handle.c test
t/rpc/log.c
t/rpc/reactor.c  # renamed to reactor_loop.c
t/rpc/reduce.c
t/rolemask/loop.c # renamed to rpc_security.c

@garlick garlick force-pushed the garlick:move_libflux_tests branch from 53d6df2 to 38f4f44 Nov 10, 2018

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Nov 10, 2018

Codecov Report

Merging #1827 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1827      +/-   ##
==========================================
- Coverage   79.76%   79.73%   -0.03%     
==========================================
  Files         197      197              
  Lines       35277    35203      -74     
==========================================
- Hits        28138    28070      -68     
+ Misses       7139     7133       -6
Impacted Files Coverage Δ
src/connectors/loop/loop.c 81.81% <ø> (-0.09%) ⬇️
src/connectors/shmem/shmem.c 84.21% <100%> (+0.36%) ⬆️
src/modules/barrier/barrier.c 76.55% <0%> (-2.07%) ⬇️
src/common/libflux/response.c 79.62% <0%> (-1.24%) ⬇️
src/cmd/flux-module.c 85.58% <0%> (-0.31%) ⬇️
src/common/libflux/message.c 81.39% <0%> (-0.25%) ⬇️
src/modules/connector-local/local.c 74.81% <0%> (ø) ⬆️
src/broker/modservice.c 80.76% <0%> (+0.96%) ⬆️

@garlick garlick force-pushed the garlick:move_libflux_tests branch from f0609ae to 9b16eba Nov 10, 2018

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Nov 10, 2018

OK, coverage is better now. I was able to drop some specialized test code from loop:// and shmem:// which is now handled in the test-only util.c.

I also tacked on a unrelated commit to add a man page for the version accessors.

@garlick garlick force-pushed the garlick:move_libflux_tests branch 3 times, most recently from 2653b22 to 2bc628f Nov 10, 2018

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Nov 12, 2018

Rebased on current master.

garlick added some commits Nov 9, 2018

testsuite: eliminate libflux unit test connector dependency
Problem: some libflux unit tests are located in the sharness
area due to their dependence on the shmem:// connector.

Add a stripped down connector implementation based on a CZMQ
"inproc" pair socket (like shmem://) used by the test server
code that does not depend on calling flux_open() and thus
can be hardwired rather than dlopened from the connector directory.

Also some cleanup: add header documentation and add a new function
test_server_environment_init() to initialize czmq zsys, then
update tests to call that.
testsuite: relocate libflux unit tests
Problem: mrpc, rpc, and rpc_chained unit tests and their
test server helper are located in the sharness area.

Now that the test server is unencumbered from its dependency
on the shmem connector, move these tests to libflux/test.

There was a name clash on the mrpc.c test, but the libflux
one only contained a couple of trivial checks, so those checks
were incorporated into the loopback one.

Partially addresses #1823
testsuite: add loopback_create()
Problem: libflux unit tests are dependent on the loop://
connector and thus cannot be located within the libflux
directory.

Add loopback_create() to the test server helper.
This creates a flux_t handle with loopback behavior
identical to the loop:// connector, except since it is
only used in unit tests, it can BAIL_OUT() on fatal
errors.

Also cleanup:  if test server is not provided with a
server callback, provide an internal one that logs all
messages received by the server.  Simplify some error
handling (use BAIL_OUT for unlikely errors).
testsuite: relocate more libflux unit tests
Problem: dispatch, handle, log, reactor, reduce,
and rolemask/loop unit tests are located in the
sharness area.

Now that the test server offers a loopback mode, these
tests that were dependent on the loop:// connector can
be moved to libflux/test.

There was a name clash on the 'handle.c' test, but the
one in libflux only had a couple of checks in it, so
those checks were pulled into the larger loopback one.

There was a name clash on the 'reactor.c' test.
The loopback one was renamed to 'reactor_loop.c'.

Renamed rolemask/loop.c to rpc_security.c.

Partially addresses #1823
testsuite: port libflux unit tests to loopback
Problem: the libflux unit tests reduce,
reactor_loop, dispatch, handle, and rpc_security
flux_open ("loop://") which depends on
src/connectors/loop being built.

Switch to loopback_create(), which is self-contained
in libflux.
connectors/loop: drop getopt/setopt
Problem: with recent changes to libflux unit tests, the
options for getting and setting test credentials in
the loop:// connector have no users.

Drop the options.  This was really only added to support
the libflux/rpc_security unit test, and now that test is
not using loop://.
connectors/shmem: drop setopt
Problem: with recent changes to libflux unit tests, the
options for setting test credentials in the shmem://
connector have no users.

This was added to support the back-to-back shmem://
connectors in libflux/util.c, but that module is now
standalone.

@garlick garlick force-pushed the garlick:move_libflux_tests branch from 2bc628f to de00d9e Nov 13, 2018

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Nov 13, 2018

Looks good to me!

@grondo grondo merged commit 2fa5cd5 into flux-framework:master Nov 13, 2018

3 checks passed

codecov/patch 100% of diff hit (target 79.76%)
Details
codecov/project Absolute coverage decreased by -0.02% but relative coverage increased by +20.23% compared to f2adaaf
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Nov 13, 2018

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.