| Message ID | 20181108041311.182694-6-amitay@ozlabs.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Test framework improvements | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | master/apply_patch Successfully applied |
| snowpatch_ozlabs/build-multiarch | success | Test build-multiarch on branch master |
On Thursday, 8 November 2018 3:13:11 PM AEDT Amitay Isaacs wrote: > Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> > --- > tests/driver.sh | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/tests/driver.sh b/tests/driver.sh > index 16d6b71..9a68305 100644 > --- a/tests/driver.sh > +++ b/tests/driver.sh > @@ -370,9 +370,9 @@ test_run () > output_mismatch=1 > fi > > + output_stderr_raw=$(cat "$stderr_file") > + output_stderr=$(cat "$stderr_file" | result_filter) > if [ $test_stderr -eq 1 ] ; then > - output_stderr_raw=$(cat "$stderr_file") > - output_stderr=$(cat "$stderr_file" | result_filter) > if [ "$output_stderr" != "$required_output_stderr" ] ; then > test_log "expected stderr:" > test_log "$required_output_stderr" > @@ -384,6 +384,11 @@ test_run () > fi > output_mismatch=1 > fi > + else > + if [ -n "$output_stderr_raw" ] ; then > + test_log "output stderr:" > + test_log "$output_stderr_raw" > + fi Would it make sense to consolidate the above logic by removing the stderr logging from the failure branch to make it bit easier to follow? It seems we could just do: test_log "output stderr:" test_log "$output_stderr" And then just output stderr_raw in case of mistmatch regardless of if we're testing stderr. In fact doesn't this always log stderr except if we're testing stderr in which case it only gets logged if that's the cause of the mismatch? Or am I missing something here? - Alistair > fi > > rm -f "$stderr_file"
On Fri, 2018-11-09 at 13:51 +1100, Alistair Popple wrote: > On Thursday, 8 November 2018 3:13:11 PM AEDT Amitay Isaacs wrote: > > Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> > > --- > > tests/driver.sh | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/tests/driver.sh b/tests/driver.sh > > index 16d6b71..9a68305 100644 > > --- a/tests/driver.sh > > +++ b/tests/driver.sh > > @@ -370,9 +370,9 @@ test_run () > > output_mismatch=1 > > fi > > > > + output_stderr_raw=$(cat "$stderr_file") > > + output_stderr=$(cat "$stderr_file" | result_filter) > > if [ $test_stderr -eq 1 ] ; then > > - output_stderr_raw=$(cat "$stderr_file") > > - output_stderr=$(cat "$stderr_file" | result_filter) > > if [ "$output_stderr" != "$required_output_stderr" ] ; > > then > > test_log "expected stderr:" > > test_log "$required_output_stderr" > > @@ -384,6 +384,11 @@ test_run () > > fi > > output_mismatch=1 > > fi > > + else > > + if [ -n "$output_stderr_raw" ] ; then > > + test_log "output stderr:" > > + test_log "$output_stderr_raw" > > + fi > > Would it make sense to consolidate the above logic by removing the > stderr > logging from the failure branch to make it bit easier to follow? It > seems we > could just do: > > test_log "output stderr:" > test_log "$output_stderr" > > And then just output stderr_raw in case of mistmatch regardless of if > we're > testing stderr. In fact doesn't this always log stderr except if > we're testing > stderr in which case it only gets logged if that's the cause of the > mismatch? > Or am I missing something here? Well the motivation to log as much information as possible if the test fails. If the test passes, then there is no reason to log extra information. From that point of view, - if the filtered output on stderr does not match * log $otuput_stderr - if the filtered output is not the same as output * log $output_stderr_raw If the test is not trying to match any output on stderr, then the output on stderr is currently ignored. This patch was added because a test was failing, but I could not figure out why. The error message was on stderr and was ignored. That's why I decided to log the $output_stderr_raw. I can make it stricter by only logging when the test has failed (mismatched rc or mismatched output_stdout). Amitay.
> If the test is not trying to match any output on stderr, then the > output on stderr is currently ignored. Fair point actually. Guess we don't really need progress bars in our log files :-) > This patch was added because a test was failing, but I could not figure > out why. The error message was on stderr and was ignored. That's why > I decided to log the $output_stderr_raw. > > I can make it stricter by only logging when the test has failed > (mismatched rc or mismatched output_stdout). > > Amitay.
diff --git a/tests/driver.sh b/tests/driver.sh index 16d6b71..9a68305 100644 --- a/tests/driver.sh +++ b/tests/driver.sh @@ -370,9 +370,9 @@ test_run () output_mismatch=1 fi + output_stderr_raw=$(cat "$stderr_file") + output_stderr=$(cat "$stderr_file" | result_filter) if [ $test_stderr -eq 1 ] ; then - output_stderr_raw=$(cat "$stderr_file") - output_stderr=$(cat "$stderr_file" | result_filter) if [ "$output_stderr" != "$required_output_stderr" ] ; then test_log "expected stderr:" test_log "$required_output_stderr" @@ -384,6 +384,11 @@ test_run () fi output_mismatch=1 fi + else + if [ -n "$output_stderr_raw" ] ; then + test_log "output stderr:" + test_log "$output_stderr_raw" + fi fi rm -f "$stderr_file"
Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> --- tests/driver.sh | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)