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

fixes for broken tests, addition to gitignore #1464

Merged
merged 9 commits into from Apr 14, 2018

Conversation

Projects
None yet
4 participants
@grondo
Copy link
Contributor

grondo commented Apr 13, 2018

This is a set of minor fixes for a couple broken sharness tests, a fix for the generation of the broker.log from test_under_flux and a small addition to .gitignore

grondo added some commits Apr 13, 2018

t/t0001-basic: fix test_size_large MIN test
Fix the test for working test_size_large, which tests MIN setting
with an invalid assumption that 123 was larger than any likely machine
nprocs+1. However, this was not true for large systems with 128 procs
for example. Instead test with an obscenely large setting for
FLUX_TEST_SIZE_MIN.

Fixes #1442
t/t2000-wreck: fix race condition in attach -n test
Problem: test for `flux-wreck attach --no-follow` has a race
condition, since there is no synchronization between when task
output makes it to kvs, and the run of `attach --no-follow`.
We still want to ensure that attach can print output that has
already made it to the kvs, so we don't want to accept only
partial output as a sign of success.

Fix: retry the `attach --no-follow` in a loop until all data has
made it into kvs.

Fixes #1459
sharness: Always generate broker.log in test_under_flux
Problem: The test_under_flux() sharness function fails to create a
broker.log for the test when the --verbose flag is used. This is
counterintuitive.

I think this was meant to send dmesg log messages to stderr when
--verbose was used. However, this no longer is the case. Additionally,
non-verbose mode used the broker's `-q, --quiet` flag, which apparently
does nothing.

Instead of the current confusing operation, just remove the use of -q,
and always write a broker.log file for tests using test_under_flux
with log-forward-level=7.
sharness: don't remove broker.log with --debug
When using test_under_flux a broker.log file is created to capture
all log messages into a log file. However, there is no way to preserve
this file for successful tests, as other test products can be
preserved with --debug.

Set the broker.log for tests to only be added to cleanup when
--debug has not been used.

Fixes #1288
gitignore: ignore code coverage byproducts
Add *.gcda and *.gcno to project .gitignore.
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Apr 13, 2018

Codecov Report

Merging #1464 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1464      +/-   ##
==========================================
- Coverage   78.73%   78.72%   -0.01%     
==========================================
  Files         163      163              
  Lines       30265    30265              
==========================================
- Hits        23828    23827       -1     
- Misses       6437     6438       +1
Impacted Files Coverage Δ
src/cmd/flux-start.c 74.19% <ø> (ø) ⬆️
src/broker/broker.c 77.3% <ø> (ø) ⬆️
src/common/libflux/keepalive.c 86.66% <0%> (-6.67%) ⬇️
src/common/libflux/ev_flux.c 97.56% <0%> (-2.44%) ⬇️
src/broker/modservice.c 79.61% <0%> (-0.98%) ⬇️
src/connectors/local/local.c 87.4% <0%> (-0.75%) ⬇️
src/common/libflux/msg_handler.c 86.64% <0%> (-0.73%) ⬇️
src/broker/overlay.c 73.81% <0%> (-0.32%) ⬇️
src/common/libflux/reactor.c 93.44% <0%> (-0.29%) ⬇️
src/common/libflux/message.c 81.25% <0%> (-0.12%) ⬇️
... and 3 more

grondo added some commits Apr 13, 2018

broker: remove -q, --quiet option
This option does nothing. Remove it to avoid confusion.

Fixes #1461
cmd/flux-start: remove reference to broker -q flag in help
Remove reference to broker -q, --quiet flag in usage output for
flux-start's `-o` option. This option is no longer supported in the
broker.
fluxometer: Don't use -q option to flux-broker
Don't try to pass '-q' option to flux-broker. The option is no longer valid.
doc: remove -q, --quiet option from flux-broker manpage
Remove the -q, --quiet option from flux-broker.adoc. It is no longer
a valid option to the broker.

@grondo grondo force-pushed the grondo:test-fixes branch from d6dae6a to e515738 Apr 13, 2018

@grondo

This comment has been minimized.

Copy link
Contributor Author

grondo commented Apr 13, 2018

I forgot that the lua tests driver fluxometer.lua also passed -q option to the broker via flux-start.
Also found that flux-start referenced -q in usage output. Fixed both of these issues and force-pushed this branch.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Apr 14, 2018

Coverage Status

Coverage decreased (-0.003%) to 79.029% when pulling e515738 on grondo:test-fixes into 0ebaf83 on flux-framework:master.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Apr 14, 2018

Thanks - looks good.

@garlick garlick merged commit 62100bf into flux-framework:master Apr 14, 2018

3 of 4 checks passed

codecov/project 78.72% (-0.01%) compared to 0ebaf83
Details
codecov/patch Coverage not affected when comparing 0ebaf83...e515738
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.003%) to 79.029%
Details
@grondo

This comment has been minimized.

Copy link
Contributor Author

grondo commented Apr 14, 2018

Thanks!

garlick added a commit to garlick/flux-sched that referenced this pull request Apr 18, 2018

sharness: update flux-sharness.sh
Problem: test_under_flux() conditionally
starts the broker with the -q option,
but that option is no longer valid.

N.B. flux-broker -q,--quiet was dropped in
PR flux-framework/flux-core#1464.

flux-sharness.sh has lagged behind the
one in flux-core.  Copy over the
flux-core version.

@grondo grondo deleted the grondo:test-fixes branch Apr 26, 2018

@grondo grondo referenced this pull request May 10, 2018

Closed

0.9.0 Release #1479

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.