From 6ccda11a98afd6f4459e9ff1c24de4ad4450de23 Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Tue, 6 Jun 2017 19:41:17 +0200 Subject: refactor: fix error handling; fix log.Origin; only one trial A bit of refactoring to fix logging and error reporting, and simplify the code. This transmogrifies some of the things committed in 0ffb41440661631fa1d520c152be4cf8ebd4c46b "Add JUnit XML reports; refactor test reporting", which did not fully match the code structuring ideas used in osmo-gsm-tester. Also solve some problems present from the start of the code base. Though this is a bit of a code bomb, it would take a lot of time to separate this into smaller bits: these changes are closely related and resulted incrementally from testing error handling and logging details. I hope it's ok. Things changed / problems fixed: Allow only a single trial to be run per cmdline invocation: unbloat trial and suite invocation in osmo-gsm-tester.py. There is a SuiteDefinition, intended to be immutable, and a mutable SuiteRun. SuiteDefinition had a list of tests, which was modified by the SuiteRun to record test results. Instead, have only the test basenames in the SuiteDefinition and create a new set of Test() instances for each SuiteRun, to ensure that no state leaks between separate suite runs. State leaking across runs can be seen in http://jenkins.osmocom.org/jenkins/view/osmo-gsm-tester/job/osmo-gsm-tester_run/453/ where an earlier sms test for sysmo succeeds, but its state gets overwritten by the later sms test for trx that fails. The end result is that both tests failed, although the first run was successful. Fix a problem with Origin: log.Origin allowed to be __enter__ed more than once, skipping the second entry. The problem there is that we'd still __exit__ twice or more, popping the Origin off the stack even though it should still remain. We could count __enter__ recurrences, but instead, completely disallow entering a second time. A code path should have one 'with' statement per object, at pivotal points like run_suites or run_tests. Individual utility functions should not do 'with' on a central object. The structure needed is, in pseudo code: try: with trial: try: with suite_run: try: with test: test_actions() The 'with' needs to be inside the 'try', so that the exception can be handled in __exit__ before it reaches the exception logging. To clarify this, like test exceptions caught in Test.run(), also move suite exception handling from Trial into SuiteRun.run_tests(). There are 'with self' in Test.run() and SuiteRun.run_tests(), which are well placed, because these are pivotal points in the main code path. Log output: clearly separate logging of distinct suites and test scripts, by adding more large_separator() calls at the start of each test. Place these separator calls in more logical places. Add separator size and spacing args. Log output: print tracebacks only once, for the test script where they happen. Have less state that duplicates other state: drop SuiteRun.test_failed_ctr and suite.test_skipped_ctr, instead add SuiteRun.count_test_results(). For test failure reporting, store the traceback text in a separate member var. In the text report, apply above changes and unclutter to achieve a brief and easy to read result overview: print less filler characters, drop the starting times, drop the tracebacks. This can be found in the individual test logs. Because the tracebacks are no longer in the text report, the suite_test.py can just print the reports and expect that output instead of asserting individual contents. In the text report, print duration in precision of .1 seconds. Add origin information and a traceback text to the junit XML result to give more context when browsing the result XML. For 'AssertionError', add the source line of where the assertion hit. Drop the explicit Failure exception. We don't need one specific exception to mark a failure, instead any arbitrary exception is treated as a failure. Use the exception's class name as fail_type. Though my original idea was to use raising exceptions as the only way to cause a test failure, I'm keeping the set_fail() function as an alternative way, because it allows test specific cleanup and may come in handy later. To have both ways integrate seamlessly, shift some result setting into 'finally' clauses and make sure higher levels (suite, trial) count the contained items' stati. Minor tweak: write the 'pass' and 'skip' reports in lower case so that the 'FAIL' stands out. Minor tweak: pass the return code that the program exit should return further outward, so that the exit(1) call does not cause a SystemExit exception to be logged. The aims of this patch are: - Logs are readable so that it is clear which logging belongs to which test and suite. - The logging origins are correct (vs. parents gone missing as previously) - A single test error does not cause following tests or suites to be skipped. - An exception "above" Exception, i.e. SystemExit and the like, *does* immediately abort all tests and suites, and the results for tests that were not run are reported as "unknown" (rather than skipped on purpose): - Raising a SystemExit aborts all. - Hitting ctrl-c aborts all. - The resulting summary in the log is brief and readable. Change-Id: Ibf0846d457cab26f54c25e6906a8bb304724e2d8 --- selftest/log_test.ok | 2 +- selftest/log_test.py | 13 +-- selftest/suite_test.ok | 101 +++++++++++++++++----- selftest/suite_test.ok.ign | 1 + selftest/suite_test.py | 18 ++-- selftest/suite_test/test_suite/test_fail_raise.py | 9 +- 6 files changed, 98 insertions(+), 46 deletions(-) (limited to 'selftest') diff --git a/selftest/log_test.ok b/selftest/log_test.ok index b9f1465..7ed94a0 100644 --- a/selftest/log_test.ok +++ b/selftest/log_test.ok @@ -38,4 +38,4 @@ nested print just prints 01:02:03 tst level2: nested l2 log() from within l3 scope [level1↪level2] [log_test.py:146] 01:02:03 tst level3: ERR: ValueError: bork [level1↪level2↪level3] [log_test.py:147: raise ValueError('bork')] - Enter the same Origin context twice -01:02:03 tst level2: nested log [level1↪level2] [log_test.py:159] +disallowed successfully diff --git a/selftest/log_test.py b/selftest/log_test.py index 46afb73..7670c8e 100755 --- a/selftest/log_test.py +++ b/selftest/log_test.py @@ -152,10 +152,13 @@ except Exception: log.log_exn() print('- Enter the same Origin context twice') -with Thing('level1'): - l2 = Thing('level2') - with l2: - with l2: - l2.log('nested log') +try: + t = Thing('foo') + with t: + with t: + raise RuntimeError('this should not be reached') +except AssertionError: + print('disallowed successfully') + pass # vim: expandtab tabstop=4 shiftwidth=4 diff --git a/selftest/suite_test.ok b/selftest/suite_test.ok index fd4d743..54c950a 100644 --- a/selftest/suite_test.ok +++ b/selftest/suite_test.ok @@ -7,7 +7,7 @@ cnf -: DBG: Found path suites_dir as [PATH]/selftest/suite_test - no suite.conf cnf -: DBG: Found path suites_dir as [PATH]/selftest/suite_test cnf empty_dir: DBG: reading suite.conf ---- [PATH]/selftest/suite_test/empty_dir/suite.conf: ERR: FileNotFoundError: [Errno 2] No such file or directory: '[PATH]/selftest/suite_test/empty_dir/suite.conf' [empty_dir↪[PATH]/selftest/suite_test/empty_dir/suite.conf] +--- [PATH]/selftest/suite_test/empty_dir/suite.conf: ERR: FileNotFoundError: [Errno 2] No such file or directory: '[PATH]/selftest/suite_test/empty_dir/suite.conf' - valid suite dir cnf -: DBG: Found path suites_dir as [PATH]/selftest/suite_test cnf test_suite: DBG: reading suite.conf @@ -24,7 +24,10 @@ resources: - run hello world test cnf -: DBG: Found config file resources.conf as [PATH]/selftest/suite_test/resources.conf in ./suite_test which is [PATH]/selftest/suite_test cnf -: DBG: Found path state_dir as [PATH]/selftest/suite_test/test_work/state_dir -tst test_suite: Suite run start + +--------------------------------------------------------------------- +trial test_suite +--------------------------------------------------------------------- tst test_suite: reserving resources in [PATH]/selftest/suite_test/test_work/state_dir ... tst test_suite: DBG: {combining='resources'} tst test_suite: DBG: {definition_conf={bts=[{'times': '1'}], ip_address=[{'times': '1'}], modem=[{'times': '2'}]}} [test_suite↪(combining_scenarios='resources')↪test_suite] @@ -49,37 +52,93 @@ tst test_suite: DBG: Picked - _hash: 19c69e45aa090fb511446bd00797690aa82ff52f ki: 47FDB2D55CE6A10A85ABDAD034A5B7B3 label: m7802 path: /wavecom_1 -tst hello_world.py:[LINENR] START [test_suite↪hello_world.py] + +---------------------------------------------- +trial test_suite hello_world.py +---------------------------------------------- tst hello_world.py:[LINENR]: hello world [test_suite↪hello_world.py:[LINENR]] tst hello_world.py:[LINENR]: I am 'test_suite' / 'hello_world.py:[LINENR]' [test_suite↪hello_world.py:[LINENR]] tst hello_world.py:[LINENR]: one [test_suite↪hello_world.py:[LINENR]] tst hello_world.py:[LINENR]: two [test_suite↪hello_world.py:[LINENR]] tst hello_world.py:[LINENR]: three [test_suite↪hello_world.py:[LINENR]] -tst hello_world.py:[LINENR] PASS [test_suite↪hello_world.py] -tst test_suite: PASS -pass: all 6 tests passed (5 skipped). +tst hello_world.py:[LINENR] Test passed (N.N sec) [test_suite↪hello_world.py] +--------------------------------------------------------------------- +trial test_suite PASS +--------------------------------------------------------------------- +PASS: test_suite (pass: 1, skip: 5) + pass: hello_world.py (N.N sec) + skip: mo_mt_sms.py + skip: mo_sms.py + skip: test_error.py + skip: test_fail.py + skip: test_fail_raise.py - a test with an error -tst test_suite: Suite run start [suite.py:[LINENR]] -tst test_error.py:[LINENR] START [test_suite↪test_error.py] [suite.py:[LINENR]] + +--------------------------------------------------------------------- +trial test_suite +--------------------------------------------------------------------- + +---------------------------------------------- +trial test_suite test_error.py +---------------------------------------------- tst test_error.py:[LINENR]: I am 'test_suite' / 'test_error.py:[LINENR]' [test_suite↪test_error.py:[LINENR]] [test_error.py:[LINENR]] -tst test_error.py:[LINENR]: ERR: AssertionError: [test_error.py:[LINENR]: assert False] -tst test_error.py:[LINENR] FAIL (AssertionError) [test_suite↪test_error.py] [suite.py:[LINENR]] -tst test_suite: FAIL [suite.py:[LINENR]] +tst test_error.py:[LINENR]: ERR: AssertionError: test_error.py:[LINENR]: assert False [test_error.py:[LINENR]] [test_suite↪test_error.py:[LINENR]] [suite.py:[LINENR]] +tst test_error.py:[LINENR]: Test FAILED (N.N sec) [test_suite↪test_error.py:[LINENR]] [suite.py:[LINENR]] +--------------------------------------------------------------------- +trial test_suite FAIL +--------------------------------------------------------------------- +FAIL: test_suite (fail: 1, skip: 5) + skip: hello_world.py (N.N sec) + skip: mo_mt_sms.py + skip: mo_sms.py + FAIL: test_error.py (N.N sec) AssertionError: test_error.py:[LINENR]: assert False [test_error.py:[LINENR]] + skip: test_fail.py + skip: test_fail_raise.py - a test with a failure -tst test_suite: Suite run start [suite.py:[LINENR]] -tst test_fail.py:[LINENR] START [test_suite↪test_fail.py] [suite.py:[LINENR]] + +--------------------------------------------------------------------- +trial test_suite +--------------------------------------------------------------------- + +---------------------------------------------- +trial test_suite test_fail.py +---------------------------------------------- tst test_fail.py:[LINENR]: I am 'test_suite' / 'test_fail.py:[LINENR]' [test_suite↪test_fail.py:[LINENR]] [test_fail.py:[LINENR]] -tst test_fail.py:[LINENR] FAIL (EpicFail) [test_suite↪test_fail.py] [suite.py:[LINENR]] -tst test_suite: FAIL [suite.py:[LINENR]] +tst test_fail.py:[LINENR]: ERR: EpicFail: This failure is expected [test_suite↪test_fail.py:[LINENR]] [suite.py:[LINENR]] +tst test_fail.py:[LINENR]: Test FAILED (N.N sec) [test_suite↪test_fail.py:[LINENR]] [suite.py:[LINENR]] +--------------------------------------------------------------------- +trial test_suite FAIL +--------------------------------------------------------------------- +FAIL: test_suite (fail: 1, skip: 5) + skip: hello_world.py (N.N sec) + skip: mo_mt_sms.py + skip: mo_sms.py + skip: test_error.py (N.N sec) + FAIL: test_fail.py (N.N sec) EpicFail: This failure is expected + skip: test_fail_raise.py - a test with a raised failure -tst test_suite: Suite run start [suite.py:[LINENR]] -tst test_fail_raise.py:[LINENR] START [test_suite↪test_fail_raise.py] [suite.py:[LINENR]] -tst test_fail_raise.py:[LINENR]: I am 'test_suite' / 'test_fail_raise.py:[LINENR]' [test_suite↪test_fail_raise.py:[LINENR]] [test_fail_raise.py:[LINENR]] -tst test_fail_raise.py:[LINENR]: ERR: Failure: ('EpicFail', 'This failure is expected') [test_fail_raise.py:[LINENR]: raise Failure('EpicFail', 'This failure is expected')] -tst test_fail_raise.py:[LINENR] FAIL (EpicFail) [test_suite↪test_fail_raise.py] [suite.py:[LINENR]] -tst test_suite: FAIL [suite.py:[LINENR]] + +--------------------------------------------------------------------- +trial test_suite +--------------------------------------------------------------------- + +---------------------------------------------- +trial test_suite test_fail_raise.py +---------------------------------------------- +tst test_fail_raise.py:[LINENR]: ERR: ExpectedFail: This failure is expected [test_fail_raise.py:[LINENR]] [test_suite↪test_fail_raise.py:[LINENR]] [suite.py:[LINENR]] +tst test_fail_raise.py:[LINENR]: Test FAILED (N.N sec) [test_suite↪test_fail_raise.py:[LINENR]] [suite.py:[LINENR]] +--------------------------------------------------------------------- +trial test_suite FAIL +--------------------------------------------------------------------- +FAIL: test_suite (fail: 1, skip: 5) + skip: hello_world.py (N.N sec) + skip: mo_mt_sms.py + skip: mo_sms.py + skip: test_error.py (N.N sec) + skip: test_fail.py (N.N sec) + FAIL: test_fail_raise.py (N.N sec) ExpectedFail: This failure is expected [test_fail_raise.py:[LINENR]] - graceful exit. diff --git a/selftest/suite_test.ok.ign b/selftest/suite_test.ok.ign index a19fb8b..dcda3b6 100644 --- a/selftest/suite_test.ok.ign +++ b/selftest/suite_test.ok.ign @@ -1,2 +1,3 @@ /[^ ]*/selftest/ [PATH]/selftest/ \.py:[0-9]* .py:[LINENR] +\([0-9.]+ sec\) (N.N sec) diff --git a/selftest/suite_test.py b/selftest/suite_test.py index 16342c5..2a92f47 100755 --- a/selftest/suite_test.py +++ b/selftest/suite_test.py @@ -20,7 +20,8 @@ assert(isinstance(s_def, suite.SuiteDefinition)) print(config.tostr(s_def.conf)) print('- run hello world test') -s = suite.SuiteRun(None, 'test_suite', s_def) +trial = log.Origin('trial') +s = suite.SuiteRun(trial, 'test_suite', s_def) results = s.run_tests('hello_world.py') print(report.suite_to_text(s)) @@ -29,26 +30,17 @@ log.style_change(src=True) print('\n- a test with an error') results = s.run_tests('test_error.py') output = report.suite_to_text(s) -assert 'FAIL: [test_suite] 1 failed ' in output -assert 'FAIL: [test_error.py]' in output -assert "type:'AssertionError' message: AssertionError()" in output -assert 'assert False' in output +print(output) print('\n- a test with a failure') results = s.run_tests('test_fail.py') output = report.suite_to_text(s) -assert 'FAIL: [test_suite] 1 failed ' in output -assert 'FAIL: [test_fail.py]' in output -assert "type:'EpicFail' message: This failure is expected" in output -assert "test.set_fail('EpicFail', 'This failure is expected')" in output +print(output) print('\n- a test with a raised failure') results = s.run_tests('test_fail_raise.py') output = report.suite_to_text(s) -assert 'FAIL: [test_suite] 1 failed ' in output -assert 'FAIL: [test_fail_raise.py]' in output -assert "type:'EpicFail' message: This failure is expected" in output -assert "raise Failure('EpicFail', 'This failure is expected')" in output +print(output) print('\n- graceful exit.') # vim: expandtab tabstop=4 shiftwidth=4 diff --git a/selftest/suite_test/test_suite/test_fail_raise.py b/selftest/suite_test/test_suite/test_fail_raise.py index a7b0b61..4e5eddb 100755 --- a/selftest/suite_test/test_suite/test_fail_raise.py +++ b/selftest/suite_test/test_suite/test_fail_raise.py @@ -1,6 +1,3 @@ -#!/usr/bin/env python3 -from osmo_gsm_tester.test import * - -print('I am %r / %r' % (suite.name(), test.name())) - -raise Failure('EpicFail', 'This failure is expected') +class ExpectedFail(Exception): + pass +raise ExpectedFail('This failure is expected') -- cgit v1.2.3