osmo-gsm-tester[master]: Add JUnit XML reports; refactor test reporting

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/.

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Thu May 18 18:37:19 UTC 2017


Patch Set 3:

(10 comments)

some more comments. I thought this would be quick, but I must see that I was wrong; the junit composition is quick, but the changes needed to handle things are more complex than I initially expected.

The comments are spread across two patch-sets, because answering to your answers; best look at the comments here on the main page with 'exand all'. Will try to stay in the newest patch set in future...

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:
> Hm I initially thought it was more interesting to know the failure rather t
ah I was assuming the failure is coming in here as an exception? ... it would be good to not have duality of setting the failure manually versus exception. The API to set a failure manually from inside a test script IMHO should raise an exception, so all failures come in here as exceptions.


Line 133:                 self.set_fail(ftype, fmsg, False)
> It looks clearer for me this way and also you have far less lines, because 
only usually one looks at try:except clauses to figure out which exceptions end up where by which exn types are in the 'except' clause. If you do 'isinstance' you're breaking the convention. Instead each 'except' could call set_fail() with different arguments (a one-liner each), and set_fail() can figure out whether to overwrite self.status or not.

I'd also like to get the NoResourceExn thing out of here, but that's for another patch I guess.

But again, cosmetics, fine for now.


Line 191:     FAIL = 'FAIL'
> I prefer to keep it this way as test has more possibilites than suite run (
it's not like it is a C enum with benefits (like warning about missing cases in a switch(){}), they are just string definitions which you are duplicating around... :P


Line 251:         self.init_test_vars()
> Yes, it is.
I see, then we should maybe fix/adjust the selftest instead? If it stays this way, please rename to 'clear_...'


https://gerrit.osmocom.org/#/c/2669/4/src/osmo_gsm_tester/suite.py
File src/osmo_gsm_tester/suite.py:

Line 32: 
* what exactly is the fail_type? is it junit specific? let's document here, or do we really need it in the first place?
* does it make sense to supply default args as empty? are we not required a type and msg to be passed always?


Line 123:             self.log_exn()
(double space but whatever)


Line 144:         if l is not None:
(again require args instead of default-empty?)


Line 182:             ret += " (%s, %d sec)" % (datetime.fromtimestamp(round(self.ts_start)).isoformat(), self.time)
clear_test_vars?


https://gerrit.osmocom.org/#/c/2669/3/src/osmo_gsm_tester/trial.py
File src/osmo_gsm_tester/trial.py:

Line 195:             elif self.status == suite.SuiteRun.UNKNOWN:
> This will be fixed when moving  run_tests to its own line above the if-else
above, you set 'self.stats = Trial.UNKNOWN', then later you compare it against 'suite.SuiteRun.UNKNOWN'? something is definitely wrong there.


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' and report.py?


-- 
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



More information about the gerrit-log mailing list