Patch Set 2: Code-Review-1 (7 comments)
https://gerrit.osmocom.org/#/c/2853/2/selftest/suite_test.py File selftest/suite_test.py: Line 33: print(output) You are not checking anymore expected output. https://gerrit.osmocom.org/#/c/2853/2/src/osmo-gsm-tester.py File src/osmo-gsm-tester.py: Line 192: if current_trial.status != trial.Trial.PASS: better return status in run_suites instead ofchecking the property afterwards. https://gerrit.osmocom.org/#/c/2853/2/src/osmo_gsm_tester/suite.py File src/osmo_gsm_tester/suite.py: Line 95: with self: This "with self" should not be here according to commit mesage. grep for "with self" to make sure there are no others still hanging there. Line 135: self.fail_tb = tb_str I think it still makes sense to use the following in case tb_str is None, for th euse case in which the test running calls set_fail(): if self.fail_tb is None: self.fail_tb = ''.join(traceback.format_stack()[:-1]).rstrip() Line 139: if (self.fail_tb): () not needed here Line 226: with self: this "with self" should not be here acording to the commit message. https://gerrit.osmocom.org/#/c/2853/2/src/osmo_gsm_tester/trial.py File src/osmo_gsm_tester/trial.py: Line 90: if exc_info[0] is None: what's going on in these lines? Trial is PASS if there is no exception? That means if an exception (failure) happens in a test but it's catched there, then the Trial still passess? Please add a comment in the code here. -- To view, visit https://gerrit.osmocom.org/2853 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf0846d457cab26f54c25e6906a8bb304724e2d8 Gerrit-PatchSet: 2 Gerrit-Project: osmo-gsm-tester Gerrit-Branch: master Gerrit-Owner: Neels Hofmeyr <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pau Espin Pedrol <[email protected]> Gerrit-HasComments: Yes
