On Wed, Apr 23, 2014 at 2:25 PM, Daniel Vetter <[email protected]> wrote: > On Wed, Apr 23, 2014 at 12:46:23PM -0400, Ilia Mirkin wrote: >> So this means that going from Skip (or NotRun) to Timeout would not be >> counted as a change (regression/etc) by summary. Is that intentional? >> >> Futhermore, I'm not 100% sure, but I think going from Pass -> Timeout >> might get considered to be a fix (and Timeout -> Pass a regression). > > Pass->Timeout usually means the kernel deadlocked, so imo should be a > regression. Might be another case where we want to treat result order > differently ...
I was pointing out how the code was going to work as written (and implicitly that I thought that was wrong, perhaps I could have been clearer). > -Daniel > >> >> I'd like to encourage you to add a few cases to summary_test.py which >> would either prove me wrong, or point out potential issues, and at >> least provide a place where you can encode your desired behaviour >> (about which we can then argue whether it's correct or not :) ) >> >> Lastly, you need to add TIMEOUT to ALL = ... at the very end of >> framework/status.py >> >> On Wed, Apr 23, 2014 at 12:29 PM, Thomas Wood <[email protected]> wrote: >> > v2: use NoChangeStatus for the timeout class >> > >> > Signed-off-by: Thomas Wood <[email protected]> >> > Cc: Daniel Vetter <[email protected]> >> > --- >> > framework/log.py | 3 ++- >> > framework/status.py | 7 +++++-- >> > framework/summary.py | 6 ++++-- >> > framework/tests/status_tests.py | 6 +++--- >> > templates/index.css | 5 ++++- >> > 5 files changed, 18 insertions(+), 9 deletions(-) >> > >> > diff --git a/framework/log.py b/framework/log.py >> > index d045847..e5154aa 100644 >> > --- a/framework/log.py >> > +++ b/framework/log.py >> > @@ -39,7 +39,8 @@ class Log(object): >> > self.__generator = (x for x in xrange(self.__total)) >> > self.__pad = len(str(self.__total)) >> > self.__summary_keys = set(['pass', 'fail', 'warn', 'crash', >> > 'skip', >> > - 'dmesg-warn', 'dmesg-fail', 'dry-run']) >> > + 'dmesg-warn', 'dmesg-fail', 'dry-run', >> > + 'timeout']) >> > self.__summary = collections.defaultdict(lambda: 0) >> > self.__lastlength = 0 >> > >> > diff --git a/framework/status.py b/framework/status.py >> > index aa42487..7e16a2b 100644 >> > --- a/framework/status.py >> > +++ b/framework/status.py >> > @@ -41,7 +41,6 @@ warn >> > dmesg-fail >> > fail >> > crash >> > -timeout >> > >> > SKIP and NOTRUN are not factored into regressions and fixes, they are >> > counted >> > seperately. They also derive from a sublcass of Status, which always >> > returns >> > @@ -63,6 +62,7 @@ __all__ = ['NOTRUN', >> > 'DMESG_WARN', >> > 'DMESG_FAIL', >> > 'SKIP', >> > + 'TIMEOUT', >> > 'ALL'] >> > >> > >> > @@ -81,7 +81,8 @@ def status_lookup(status): >> > 'crash': CRASH, >> > 'dmesg-warn': DMESG_WARN, >> > 'dmesg-fail': DMESG_FAIL, >> > - 'notrun': NOTRUN} >> > + 'notrun': NOTRUN, >> > + 'timeout': TIMEOUT} >> > >> > try: >> > return status_dict[status] >> > @@ -211,6 +212,8 @@ class NoChangeStatus(Status): >> > >> > NOTRUN = NoChangeStatus('Not Run') >> > >> > +TIMEOUT = NoChangeStatus('timeout') >> > + >> > SKIP = NoChangeStatus('skip') >> > >> > PASS = Status('pass', 0, (1, 1)) >> > diff --git a/framework/summary.py b/framework/summary.py >> > index 47138bf..9228330 100644 >> > --- a/framework/summary.py >> > +++ b/framework/summary.py >> > @@ -342,8 +342,9 @@ class Summary: >> > Private: Find the total number of pass, fail, crash, skip, and >> > warn in >> > the *last* set of results stored in self.results. >> > """ >> > - self.totals = {'pass': 0, 'fail': 0, 'crash': 0, 'skip': 0, >> > 'warn': 0, >> > - 'dmesg-warn': 0, 'dmesg-fail': 0} >> > + self.totals = {'pass': 0, 'fail': 0, 'crash': 0, 'skip': 0, >> > + 'timeout': 0, 'warn': 0, 'dmesg-warn': 0, >> > + 'dmesg-fail': 0} >> > >> > for test in self.results[-1].tests.itervalues(): >> > self.totals[str(test['result'])] += 1 >> > @@ -472,6 +473,7 @@ class Summary: >> > " fail: {fail}\n" >> > " crash: {crash}\n" >> > " skip: {skip}\n" >> > + " timeout: {timeout}\n" >> > " warn: {warn}\n" >> > " dmesg-warn: {dmesg-warn}\n" >> > " dmesg-fail: {dmesg-fail}".format(**self.totals)) >> > diff --git a/framework/tests/status_tests.py >> > b/framework/tests/status_tests.py >> > index c1d8c86..7b1b30a 100644 >> > --- a/framework/tests/status_tests.py >> > +++ b/framework/tests/status_tests.py >> > @@ -44,7 +44,7 @@ FIXES = list(itertools.combinations(reversed(STATUSES), >> > 2)) + \ >> > list(itertools.combinations(list(reversed(PROBLEMS)) + ["skip"], >> > 2)) >> > >> > # The statuses that don't cause changes when transitioning from one >> > another >> > -NO_OPS = ('skip', 'notrun') >> > +NO_OPS = ('skip', 'notrun', 'timeout') >> > >> > >> > def initialize_status(): >> > @@ -74,7 +74,7 @@ def test_gen_lookup(): >> > """ Generator that attempts to do a lookup on all statuses """ >> > yieldable = check_lookup >> > >> > - for stat in STATUSES + ['skip', 'notrun']: >> > + for stat in STATUSES + ['skip', 'notrun', 'timeout']: >> > yieldable.description = "Lookup: {}".format(stat) >> > yield yieldable, stat >> > >> > @@ -149,7 +149,7 @@ def check_not_change(new, old): >> > >> > >> > def test_not_change(): >> > - """ Skip and NotRun should not count as changes """ >> > + """ Skip, NotRun and Timeout should not count as changes """ >> > yieldable = check_not_change >> > >> > for nochange, stat in itertools.permutations(NO_OPS, 2): >> > diff --git a/templates/index.css b/templates/index.css >> > index 577370c..3389738 100644 >> > --- a/templates/index.css >> > +++ b/templates/index.css >> > @@ -36,7 +36,7 @@ td:first-child > div { >> > background-color: #c8c838 >> > } >> > >> > -td.skip, td.warn, td.fail, td.pass, td.trap, td.abort, td.crash, >> > td.dmesg-warn, td.dmesg-fail { >> > +td.skip, td.warn, td.fail, td.pass, td.trap, td.abort, td.crash, >> > td.dmesg-warn, td.dmesg-fail, td.timeout { >> > text-align: right; >> > } >> > >> > @@ -67,6 +67,9 @@ tr:nth-child(even) td.fail { background-color: #e00505; >> > } >> > tr:nth-child(odd) td.dmesg-fail { background-color: #ff2020; } >> > tr:nth-child(even) td.dmesg-fail { background-color: #e00505; } >> > >> > +tr:nth-child(odd) td.timeout { background-color: #83bdf6; } >> > +tr:nth-child(even) td.timeout { background-color: #4a9ef2; } >> > + >> > tr:nth-child(odd) td.trap { background-color: #111111; } >> > tr:nth-child(even) td.trap { background-color: #000000; } >> > tr:nth-child(odd) td.abort { background-color: #111111; } >> > -- >> > 1.9.0 >> > >> > _______________________________________________ >> > Piglit mailing list >> > [email protected] >> > http://lists.freedesktop.org/mailman/listinfo/piglit >> _______________________________________________ >> Piglit mailing list >> [email protected] >> http://lists.freedesktop.org/mailman/listinfo/piglit > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
