This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Pau Espin Pedrol gerrit-no-reply at lists.osmocom.orgPatch Set 3: (13 comments) https://gerrit.osmocom.org/#/c/2669/3/src/osmo_gsm_tester/suite.py File src/osmo_gsm_tester/suite.py: Line 124: if self.status == Test.UNKNOWN: > ah I was assuming the failure is coming in here as an exception? ... it wou Handling this duality is not a big issue and provides some way for tests to mark it as failure and continue cleaning/processing whatever if needed afterwards. Line 133: self.set_fail(ftype, fmsg, False) > only usually one looks at try:except clauses to figure out which exceptions I'm not braking that convention, as expressed in "except Exception", all Exception are catched. I just check the type to improve the information provided on the message. Line 191: FAIL = 'FAIL' > it's not like it is a C enum with benefits (like warning about missing case Well as they are part of class, then it's clear status returned by that class can be of one of the status defined inside the same class. This way we easily now suite_run() cannot return SKIP for instance. https://gerrit.osmocom.org/#/c/2669/5/src/osmo_gsm_tester/suite.py File src/osmo_gsm_tester/suite.py: Line 182: ret += " (%s, %d sec)" % (datetime.fromtimestamp(round(self.ts_start)).isoformat(), self.time) > actually, just 'clear()' or rather 'start()' would be an even better name? I like the current name, but if you prefer one of those I'll change it. https://gerrit.osmocom.org/#/c/2669/5/src/osmo_gsm_tester/trial.py File src/osmo_gsm_tester/trial.py: Line 193: if suite_run.run_tests(names) == suite.SuiteRun.FAIL: > (IIRC you agreed to take it out to a separate line but it's still in the 'i Sorry I forgot to apply this change, I'll do now. Too much code refactoring made me it missing this. https://gerrit.osmocom.org/#/c/2669/4/src/osmo_gsm_tester/trial_report.py File src/osmo_gsm_tester/trial_report.py: Line 26 > since it reports on trial, suites and tests, maybe call this just 'Report' OK https://gerrit.osmocom.org/#/c/2669/5/src/osmo_gsm_tester/trial_report.py File src/osmo_gsm_tester/trial_report.py: Line 27 > there is a mix here of using self.trial vs. passing trial as an argument. N Ok I'll remove the classes. I prefer to have all of them top level to be able to re-use them if needed to print a suite. For instance for the text one it is used in selftest. Line 32 > this composes the same path as below, which is a) a source of errors and b) Sorry, this should not be there indeed. Line 34 > should we really log the junit XML markup to the log file? if yes, please c The log function is intended to explicitly log stuff so it makes no sense to remove the log or make it dbg I think. It is not used right now but as I was already writing it for TestReportText class I also put it here for completeness. As it's not used I'll remove it. Line 37 > I think trial.py should feed the file name. OK Line 38 > I think trial.py should do this logging. This code should strictly compose OK Line 75 > (same as for TrialReportJunit above applies here) OK Line 79 > Osmocom code usually avoids double blank lines *everywhere*. OK -- To view, visit https://gerrit.osmocom.org/2669 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iedf6d912b3cce3333a187a4ac6d5c6b70fe9d5c5 Gerrit-PatchSet: 3 Gerrit-Project: osmo-gsm-tester Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de> Gerrit-HasComments: Yes