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

build/test: fix asciidoc build in travis and run_timeout() macro + misc test fixes #2104

Merged
merged 8 commits into from Mar 26, 2019

Conversation

@garlick
Copy link
Member

commented Mar 25, 2019

This is an assortment of build/test bug fixes for things @grondo and I stumbled over this morning.

There is also a bit of gratuitous cleanup in t/Makefile.am to avoid listing test scripts twice.

@grondo grondo force-pushed the garlick:misc_fixes branch from c67db10 to 0fc3209 Mar 25, 2019
@garlick

This comment has been minimized.

Copy link
Member Author

commented Mar 25, 2019

Hey @chu11, how do you like this PR? @grondo and I are at least nominally disqualified from pressing the button...

garlick and others added 8 commits Mar 25, 2019
Problem: make distcheck fails because doc/man3/Makefile.am
refers to flux_kvs_eventlog_update.3, which was removed
from flux_kvs_eventlog_create.adoc earlier.

Drop this reference.

Fixes #2100
Problem: sharness run_timeout() succeeds even if called
program does not exist.

Apparently one must check the return value of exec()
and fail the one-line perl script if exec() fails.

Fixes #2103
Problem: The check for asciidoctor version >= 1.5.7 was flawed in
some way, and failed to grab the version in some or all circumstances.
This caused the build of docs to be silently disabled in Travis-CI
for example.

Simplify the sed(1) expression used to grab the asciidoctor version
for `asciidoctor --version` output (tested working). Furthermore,
add a surrounding AC_MSG_CHECKING/AC_MSG_RESULT block so that the
detected version is printed in output for aid in future debugging
efforts.

Fixes #2102
If --enable-docs or --enable-docs=yes is used in configure, fail
./configure with an error when no asciidoc generator is found,
instead of auto-disabling documentation.

This provides a mode to build docs or fail during CI runs or
package building, instead of silently disabling documention.
Always add --enable-docs to travis-ci builds to force an error
if, for some reason, an asciidoc formatter is not found (instead
of silently disabling docs)
Problem: scripts are repeated in t/Makefile.am, and
some scripts are included in the dist via EXTRA_DIST,
and others via dist_check_SCRIPTS.

Drop scripts from EXTRA_DIST and put all scripts in
dist_check_SCRIPTS.  Factor out TESTSCRIPTS, and include
that in both TESTS (the scripts and programs that are
run as TAP tests) and dist_check_SCRIPTS.

Also for clarity:
- move clean-local target up with other un-magical make targets.
- Move check_PROGRAMS += addition near check_PROGRAMS.

Fixes #2101
In the test for job-manager.exec-hello failure when jobs still exist
with outstanding start requests, a `run_timeout` was used to execute
a `test_must_fail` shell function, which clearly cannot work.

Since we still want to be able to time out the script that is trying
to register with the job-manager (since a failure would hang forever),
use `test_expect_code 1` to differentiate between a timeout and
an expected failure.
Add a test to t0001-basic.t that verifies that run_timeout()
fails if its 2nd argument cannot be executed.
@garlick garlick force-pushed the garlick:misc_fixes branch from 0fc3209 to 926a400 Mar 25, 2019
@garlick

This comment has been minimized.

Copy link
Member Author

commented Mar 25, 2019

Rebased on current master.

@grondo grondo referenced this pull request Mar 25, 2019
@codecov-io

This comment has been minimized.

Copy link

commented Mar 25, 2019

Codecov Report

Merging #2104 into master will increase coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2104      +/-   ##
==========================================
+ Coverage    80.3%   80.36%   +0.06%     
==========================================
  Files         196      196              
  Lines       31320    31320              
==========================================
+ Hits        25150    25169      +19     
+ Misses       6170     6151      -19
Impacted Files Coverage Δ
src/cmd/flux-event.c 77.97% <0%> (ø) ⬆️
src/common/libflux/reactor.c 92.08% <0%> (+0.22%) ⬆️
src/common/libflux/message.c 81.88% <0%> (+0.24%) ⬆️
src/modules/connector-local/local.c 74.66% <0%> (+0.29%) ⬆️
src/connectors/local/local.c 89.28% <0%> (+0.71%) ⬆️
src/common/libflux/msg_handler.c 90.23% <0%> (+1.17%) ⬆️
src/common/libflux/mrpc.c 88.93% <0%> (+1.18%) ⬆️
src/common/libflux/ev_flux.c 100% <0%> (+2.43%) ⬆️
src/modules/job-manager/start.c 72.56% <0%> (+3.53%) ⬆️
src/common/libutil/aux.c 94.44% <0%> (+3.7%) ⬆️
@grondo

This comment has been minimized.

Copy link
Contributor

commented Mar 25, 2019

Actually I have one other fix that could be pushed here if that is OK @garlick.

I noticed many sharness tests are missing this code to append --logfile to sharness when run with FLUX_TESTS_LOGFILE

# Append --logfile option if FLUX_TESTS_LOGFILE is set in environment:
test -n "$FLUX_TESTS_LOGFILE" && set -- "$@" --logfile

We could update the missing tests here. This really only affects Travis which runs with FLUX_TESTS_LOGFILE and results in the verbose test output going to $(basename $0).OUTPUT, so the impact is pretty low if you'd rather skip it for now.

@garlick

This comment has been minimized.

Copy link
Member Author

commented Mar 25, 2019

Makes sense to add that here IMHO.

@grondo

This comment has been minimized.

Copy link
Contributor

commented Mar 25, 2019

hm, actually test_under_flux adds this option so maybe no problem here... ignore my previous comment.

@chu11

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2019

everything lgtm

@chu11 chu11 merged commit 5fd4a83 into flux-framework:master Mar 26, 2019
4 checks passed
4 checks passed
Mergify — Summary 1 potential rule
Details
codecov/patch Coverage not affected when comparing 6833978...926a400
Details
codecov/project 80.36% (+0.06%) compared to 6833978
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@garlick

This comment has been minimized.

Copy link
Member Author

commented Mar 26, 2019

Thanks!

@garlick garlick deleted the garlick:misc_fixes branch Apr 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.