Patch Set 2: (6 comments)
some replies here, more in form of the next patch set 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. by printing the output, all of it is verified in the suite_test.ok file 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 afterwar why? I want the trial's status property to reflect the result in all cases; no need to add a separate way to return the result. https://gerrit.osmocom.org/#/c/2853/2/src/osmo_gsm_tester/suite.py File src/osmo_gsm_tester/suite.py: Line 135: self.fail_tb = tb_str > I think it still makes sense to use the following in case tb_str is None, f ok, nice idea (why the rstrip() though?) Line 136: log.garble = True oops, here is a debug nonsense that leaked into the patch Line 139: if (self.fail_tb): > () not needed here indeed 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? Ah I notice that none of this is actually needed. We have already evaluated the results before Trial.__exit__(), all we will catch here is the SystemExit from the exit(1) call. I cleaned up some more... -- 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: Neels Hofmeyr <[email protected]> Gerrit-Reviewer: Pau Espin Pedrol <[email protected]> Gerrit-HasComments: Yes
