On 09/18/2017 02:19 PM, Josef Bacik wrote: > On Mon, Sep 18, 2017 at 01:48:31PM -0600, Shuah Khan wrote: >> On 09/18/2017 12:24 PM, Josef Bacik wrote: >>> On Mon, Sep 18, 2017 at 12:13:40PM -0600, Shuah Khan wrote: >>>> On 09/18/2017 11:52 AM, Josef Bacik wrote: >>>>> On Mon, Sep 18, 2017 at 11:46:18AM -0600, Shuah Khan wrote: >>>>>> On 09/18/2017 11:37 AM, josef@xxxxxxxxxxxxxx wrote: >>>>>>> From: Josef Bacik <jbacik@xxxxxx> >>>>>>> >>>>>>> Some of the networking tests are very noisy and make it impossible to >>>>>>> see if we actually passed the tests as they run. Default to suppressing >>>>>>> the output from any tests run in order to make it easier to track what >>>>>>> failed. >>>>>>> >>>>>>> Signed-off-by: Josef Bacik <jbacik@xxxxxx> >>>>>>> -- >>>>>> >>>>>> This change suppresses pass/fail wrapper output for all tests, not just the >>>>>> networking tests. >>>>>> >>>>>> Could you please send me before and after results for what you are trying >>>>>> to fix. >>>>>> >>>>> >>>>> Yeah I wanted to suppress extraneous output from everybody, I just happened to >>>>> notice it because I was testing net. The default thing already spits out what >>>>> it's running and pass/fail, there's no need to include all of the random output >>>>> unless the user wants to go and run the test manually. As it is now it's >>>>> _impossible_ to tell what ran and what passed/failed because of all the random >>>>> output. >>>> >>>> Unfortunately kselftests have lots of users that want different things. A recent >>>> request is to use TAP13 format for output for external parsers to be able to parse. >>>> That is what this change to add TAP13 header does. >>>> >>>> The output you are seeing is the TAP 13 format to indicate the test has passed. >>>> >>>> The right fix would be to suppress the Pass/Fail from the individual shell script >>>> and have the shell script exit with error code. kselftest lib.mk will handle the >>>> error code and print out pass/fail like it is doing now. >>>> >>>> Using the common logic will help avoid duplicate code in tests/test scripts and >>>> also makes the pass/fail messages consistent. >>>> >>>> In the following output the individual test output can be eliminated since lib.mk >>>> run_tests does that for you. In addition, you will also get a count of tests at >>>> the end of the run of all tests in a test directory. >>>> >>>> TAP version 13 >>>> selftests: run_netsocktests >>>> ======================================== >>>> -------------------- >>>> running socket test >>>> -------------------- >>>> [PASS] >>>> ok 1..1 selftests: run_netsocktests [PASS] >>>> selftests: run_afpackettests >>>> ======================================== >>>> must be run as root >>>> ok 1..2 selftests: run_afpackettests [PASS] >>>> selftests: test_bpf.sh >>>> ======================================== >>>> test_bpf: [FAIL] >>>> not ok 1..3 selftests: test_bpf.sh [FAIL] >>>> selftests: netdevice.sh >>>> ======================================== >>>> SKIP: Need root privileges >>>> ok 1..4 selftests: netdevice.sh [PASS] >>>> >>>> If you eliminate that you will just see the common lib.mk results. >>>> >>>> TAP version 13 >>>> selftests: run_netsocktests >>>> ======================================== >>>> ok 1..1 selftests: run_netsocktests [PASS] >>>> selftests: run_afpackettests >>>> ======================================== >>>> must be run as root >>>> ok 1..2 selftests: run_afpackettests [PASS] >>>> ======================================== >>>> selftests: test_bpf.sh >>>> ======================================== >>>> not ok 1..3 selftests: test_bpf.sh [FAIL] >>>> selftests: netdevice.sh >>>> ======================================== >>>> SKIP: Need root privileges >>>> ok 1..4 selftests: netdevice.sh [PASS] >>>> >>>> >>>> If you would like to fix the duplicate output, please send me patches >>>> to remove pass/fail output strings from tests instead. It is on my >>>> todo to do that this release. >>>> >>> >>> I'm confused, this is exactly what my patch does, it strips all of the >>> extraneous output and leaves only the TAP13 output. Here is the output without >>> my suppression patch >>> >>> https://da.gd/pup0 >>> >>> and here is the output with my suppression patch >>> >>> https://da.gd/3olKj >>> >>> Unless I'm missing something subtle it appears to be exactly the output you >>> want, without the random crap from the other tests. The only thing I'm >>> redirecting is the output of the _test_ itself, $$BASENAME_TEST from what I can >>> tell is the actual test we're running, not the wrapper, so everything is as it >>> should be. Thanks, >>> >>> Josef >>> >> >> Yes you are right. Yes I see that you are redirecting all >> output from the test with (./$$BASENAME_TEST > /dev/null 2>&1) >> My bad. >> >> cd `dirname $$TEST` > /dev/null; (./$$BASENAME_TEST > /dev/null 2>&1 && echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; cd - > /dev/null;\ >> >> However, it also suppresses important error messages from individual tests >> like in the case of rtnetlink.sh from your before and after output. >> >> User knows the test failed, but doesn't know why. One way to solve >> the problem is creating a temp file with the output for reference. >> Something like: >> >> (./$$BASENAME_TEST > /tmp/kselftest.out 2>&1 >> >> > > So I didn't do a global file because we'd have to append to it to make sure we > didn't lose any history. If this doesn't bother you I can do that instead, and > then just remove the file whenever we start running things. Let me know if > that's what you would prefer. Thanks, > > Josef > Right. Let's not worry about global file. Let's just use /tmp/$$BASENAME_TEST for now. Something like the below diff: diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk index 223234cd98e9..317393e430c9 100644 --- a/tools/testing/selftests/lib.mk +++ b/tools/testing/selftests/lib.mk @@ -24,7 +24,8 @@ define RUN_TESTS echo "selftests: Warning: file $$BASENAME_TEST is not executable, correct this.";\ echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; \ else \ - cd `dirname $$TEST` > /dev/null; (./$$BASENAME_TEST > /dev/null 2>&1 && echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; cd - > /dev/null;\ + echo "Please check /tmp/$$BASENAME_TEST for full test output"; \ + cd `dirname $$TEST` > /dev/null; (./$$BASENAME_TEST > /tmp/$$BASENAME_TEST && echo "ok 1..$$test_num selftests: $$BASENAME_TEST [PASS]") || echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; cd - > /dev/null;\ fi; \ done; endef thanks, -- Shuah -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
