From eb92f7bdd615cb2c67066d6515848af99266e13c Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Fri, 13 Apr 2018 10:48:01 -0700 Subject: [PATCH 1/9] 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/t0001-basic.t | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t0001-basic.t b/t/t0001-basic.t index 7c8ef689ecfc..a991c6035e8c 100755 --- a/t/t0001-basic.t +++ b/t/t0001-basic.t @@ -308,8 +308,8 @@ test_expect_success 'builtin test_size_large () works' ' test -n "$size" && size=$(FLUX_TEST_SIZE_MAX=2 test_size_large) && test "$size" = "2" && - size=$(FLUX_TEST_SIZE_MIN=123 FLUX_TEST_SIZE_MAX=1000 test_size_large) && - test "$size" = "123" + size=$(FLUX_TEST_SIZE_MIN=12345 FLUX_TEST_SIZE_MAX=23456 test_size_large) && + test "$size" = "12345" ' waitfile=${SHARNESS_TEST_SRCDIR}/scripts/waitfile.lua From c9d222e1d034c252ef5fc42a34393e857ab2ef05 Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Fri, 13 Apr 2018 11:03:08 -0700 Subject: [PATCH 2/9] 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 --- t/t2000-wreck.t | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/t/t2000-wreck.t b/t/t2000-wreck.t index 4ba374420b42..bac86ac1cb88 100755 --- a/t/t2000-wreck.t +++ b/t/t2000-wreck.t @@ -350,14 +350,24 @@ test_expect_success 'flux-wreck: attach --label-io' ' test_expect_success 'wreck: attach --no-follow works' ' flux wreckrun -d -l -n4 sh -c "echo before; sleep 30; echo after" && test_when_finished flux wreck kill $(last_job_id) && - run_timeout 5 flux wreck attach --no-follow $(last_job_id) >output.attach-n && cat >expected.attach-n <<-EOF && before before before before EOF - test_cmp expected.attach-n output.attach-n + i=0 && + while test $i -lt 3; do + run_timeout 5 \ + flux wreck attach --no-follow $(last_job_id) >output.attach-n + test_cmp expected.attach-n output.attach-n + rc=$? + if test $rc = 0; then break; fi + # retry + i=$((i+1)) + sleep 0.1 + done && + test $rc -eq 0 ' test_expect_success 'wreck: dumplog works' ' test_must_fail flux wreckrun --input=bad.file hostname && From 51447097188c73a8baef2b96d055b902556127aa Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Fri, 13 Apr 2018 15:06:28 -0700 Subject: [PATCH 3/9] 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. --- t/sharness.d/flux-sharness.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/t/sharness.d/flux-sharness.sh b/t/sharness.d/flux-sharness.sh index 3f16388cb745..34aeaaf17de3 100644 --- a/t/sharness.d/flux-sharness.sh +++ b/t/sharness.d/flux-sharness.sh @@ -70,10 +70,8 @@ test_under_flux() { cleanup check_module_list return fi - quiet="-o -q,-Slog-filename=${log_file},-Slog-forward-level=7" if test "$verbose" = "t" -o -n "$FLUX_TESTS_DEBUG" ; then flags="${flags} --verbose" - quiet="" fi if test "$debug" = "t" -o -n "$FLUX_TESTS_DEBUG" ; then flags="${flags} --debug" @@ -105,9 +103,10 @@ test_under_flux() { unset FLUX_RC3_PATH fi + logopts="-o -Slog-filename=${log_file},-Slog-forward-level=7" TEST_UNDER_FLUX_ACTIVE=t \ TERM=${ORIGINAL_TERM} \ - exec flux start --bootstrap=selfpmi --size=${size} ${quiet} ${timeout} \ + exec flux start --bootstrap=selfpmi --size=${size} ${logopts} ${timeout} \ "sh $0 ${flags}" } From 812b56e474b27e4f5170bc56a6ffe62217f63de8 Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Fri, 13 Apr 2018 15:12:30 -0700 Subject: [PATCH 4/9] 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 --- t/sharness.d/flux-sharness.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/sharness.d/flux-sharness.sh b/t/sharness.d/flux-sharness.sh index 34aeaaf17de3..8412a0c51612 100644 --- a/t/sharness.d/flux-sharness.sh +++ b/t/sharness.d/flux-sharness.sh @@ -65,7 +65,7 @@ test_under_flux() { personality=${2:-full} log_file="$TEST_NAME.broker.log" if test -n "$TEST_UNDER_FLUX_ACTIVE" ; then - cleanup rm "${SHARNESS_TEST_DIRECTORY:-..}/$log_file" + test "$debug" = "t" || cleanup rm "${SHARNESS_TEST_DIRECTORY:-..}/$log_file" flux_module_list > module-list.initial cleanup check_module_list return From cfd62e7c2f99021af863fdde46388018f3388b53 Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Fri, 13 Apr 2018 15:24:06 -0700 Subject: [PATCH 5/9] gitignore: ignore code coverage byproducts Add *.gcda and *.gcno to project .gitignore. --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index af23eb60dc76..7563df4eaf99 100644 --- a/.gitignore +++ b/.gitignore @@ -57,6 +57,9 @@ libltdl/ *.hex *.pyc *.pyo +# gcov output +*.gcno +*.gcda # autoconf-preprocessed Makefile From 1704e6fa4b1d04a48c2014882fbb16a4899457ba Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Fri, 13 Apr 2018 16:28:51 -0700 Subject: [PATCH 6/9] 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. --- src/cmd/flux-start.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmd/flux-start.c b/src/cmd/flux-start.c index c0496b350e60..a2502d42ec62 100644 --- a/src/cmd/flux-start.c +++ b/src/cmd/flux-start.c @@ -100,7 +100,7 @@ static struct optparse_option opts[] = { .usage = "Set number of ranks in new instance", }, { .name = "broker-opts",.key = 'o', .has_arg = 1, .arginfo = "OPTS", .flags = OPTPARSE_OPT_AUTOSPLIT, - .usage = "Add comma-separated broker options, e.g. \"-o,-q\"", }, + .usage = "Add comma-separated broker options, e.g. \"-o,-v\"", }, { .name = "killer-timeout",.key = 'k', .has_arg = 1, .arginfo = "SECONDS", .usage = "After a broker exits, kill other brokers after SECONDS", }, { .name = "trace-pmi-server", .has_arg = 0, .arginfo = NULL, From 8c3deffa9e69055f899cd4e1bbb1810cf02c01a3 Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Fri, 13 Apr 2018 16:30:00 -0700 Subject: [PATCH 7/9] 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. --- t/fluxometer.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/fluxometer.lua b/t/fluxometer.lua index 1226db649a82..15e6cf0ffe4c 100644 --- a/t/fluxometer.lua +++ b/t/fluxometer.lua @@ -140,7 +140,7 @@ function fluxTest.init (...) end test.log_file = "lua-"..test.prog..".broker.log" - test.start_args = { "-o,-q,-Slog-filename=" .. test.log_file } + test.start_args = { "-o,-Slog-filename=" .. test.log_file } local path = fluxTest.fluxbindir .. "/flux" local mode = posix.stat (path, 'mode') From 7e4592d69366d0f85dcec4522654d50d844b82e2 Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Fri, 13 Apr 2018 15:56:30 -0700 Subject: [PATCH 8/9] broker: remove -q, --quiet option This option does nothing. Remove it to avoid confusion. Fixes #1461 --- src/broker/broker.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/broker/broker.c b/src/broker/broker.c index 994a06a487b2..2acaba209a0a 100644 --- a/src/broker/broker.c +++ b/src/broker/broker.c @@ -184,10 +184,9 @@ static const struct flux_handle_ops broker_handle_ops; static int exit_rc = 0; -#define OPTIONS "+vqM:X:k:s:g:EIS:" +#define OPTIONS "+vM:X:k:s:g:EIS:" static const struct option longopts[] = { {"verbose", no_argument, 0, 'v'}, - {"quiet", no_argument, 0, 'q'}, {"security", required_argument, 0, 's'}, {"module-path", required_argument, 0, 'X'}, {"k-ary", required_argument, 0, 'k'}, @@ -202,7 +201,6 @@ static void usage (void) fprintf (stderr, "Usage: flux-broker OPTIONS [initial-command ...]\n" " -v,--verbose Be annoyingly verbose\n" -" -q,--quiet Be mysteriously taciturn\n" " -X,--module-path PATH Set module search path (colon separated)\n" " -s,--security=plain|curve|none Select security mode (default: curve)\n" " -k,--k-ary K Wire up in a k-ary tree\n" @@ -238,8 +236,6 @@ void parse_command_line_arguments(int argc, char *argv[], case 'v': /* --verbose */ ctx->verbose = true; break; - case 'q': /* --quiet */ - break; case 'X': /* --module-path PATH */ if (attr_set (ctx->attrs, "conf.module_path", optarg, true) < 0) log_err_exit ("setting conf.module_path attribute"); From e515738c40e27484251b905d9e15af19cbfa65f0 Mon Sep 17 00:00:00 2001 From: "Mark A. Grondona" Date: Fri, 13 Apr 2018 15:58:18 -0700 Subject: [PATCH 9/9] 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. --- doc/man1/flux-broker.adoc | 3 --- 1 file changed, 3 deletions(-) diff --git a/doc/man1/flux-broker.adoc b/doc/man1/flux-broker.adoc index e93829b0fb3b..8dca5f3d153f 100644 --- a/doc/man1/flux-broker.adoc +++ b/doc/man1/flux-broker.adoc @@ -50,9 +50,6 @@ Summarize available options. *-v, --verbose*:: Be annoyingly chatty. -*-q, --quiet*:: -Suppress messages intended for interactive users. - *-S, --setattr*='ATTR=VAL':: Set initial value for broker attribute.