On 23 April 2014 19:29, Ilia Mirkin <[email protected]> wrote: > 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 :) )
Having looked again at how NoChangeStatus works, it is probably not the correct class for timeout and instead it should use a normal Status class. If placed just above the "crash" status this would mean most status changes to timeout would be a regression. >>> >>> 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
