Patch Set 3:
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
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.
Line 182: ret += " (%s, %d sec)" %
> 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.
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.
> since it reports on trial, suites and tests, maybe call this just 'Report'
> 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.
> this composes the same path as below, which is a) a source of errors and b)
Sorry, this should not be there indeed.
> 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.
> I think trial.py should feed the file name.
> I think trial.py should do this logging. This code should strictly compose
> (same as for TrialReportJunit above applies here)
> Osmocom code usually avoids double blank lines *everywhere*.
To view, visit https://gerrit.osmocom.org/2669
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-Owner: Pau Espin Pedrol <pes...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de>
Gerrit-Reviewer: neels <nhofm...@sysmocom.de>