http://trac.webkit.org/changeset/133380 changed the output from "no expected results found" to "missing results", but _parseNewRunWebKitTestsOutput() in master.cfg still uses the old format: http://trac.webkit.org/browser/trunk/Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg?rev=133299#L365 It is similar breakage to https://bugs.webkit.org/show_bug.cgi?id=93537 I think the best way to fix it is moving the output parsing to NRWT and dumping only one, easily parsable, never changing line for master.cfg.
Argh, I checked the parsing code for the chromium bots but forgot that the webkit.org bots might be using a different parsing algorithm :(. I don't think it's reasonable to expect the output to be "never changing", but it makes sense to adopt something easier to parse, that's for sure. I've often thought the thing to do is to echo a subset of results.json (or even all of it, maybe) to stdout and just parse that. We should also add actual tests that check the format matches what we expect somewhere, and add comments about what needs to be checked if those tests start to fail. I have at least one fixme to do this in the printing_unittest.py, I think.
Are you going to fix this annoying regression?
Oh, I'm sorry, I thought this had been immediately fixed. It must've fallen off my radar somehow. Yes, I'll look into fixing this today. As to the longer-term fix, I've just landed a long series of patches that get us much closer to the point of producing easily parsable output. I'll probably look into doing that next week.
Okay, I thought this had been fixed ... were the changes in bug 93537 and bug 93789 not sufficient? What's still broken?
(In reply to comment #4) > Okay, I thought this had been fixed ... were the changes in bug 93537 and bug 93789 not sufficient? What's still broken? No, it isn't fixed. It is a new regression caused by r133380. Previously a new test was reported as: "Regressions: Unexpected no expected results found : (1)", and "Regressions: Unexpected missing results (1)" after r133380. Previously a new test without expected result wasn't calculated as failure and wasn't make the bot red if there was only new tests and no other regressions. The bug is somewhere in _parseNewRunWebKitTestsOutput and/or evaluateCommand.
Here is an output where you can test these parser functions - http://build.webkit.sed.hu/builders/x86-32%20Linux%20Qt%20Release%20NRWT/builds/25127/steps/layout-test/logs/stdio (It is an artificial example, because I removed the expected file on the bot manually to produce the buggy output for you to help debugging.)
Created attachment 178871 [details] Patch
Comment on attachment 178871 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=178871&action=review > Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:82 > +Regressions: Unexpected missing results (1) > + svg/custom/zero-path-square-cap-rendering2.svg [ Missing ] We probably shouldn't report missing results as "Regressions" if tests just had been added.
Comment on attachment 178871 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=178871&action=review > Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:371 > - ('flakes', re.compile(r'[Uu]nexpected flakiness.+:?\s*\((\d+)\)')), > + ('flakes', re.compile(r'Unexpected flakiness.+\((\d+)\)')), > ('new passes', re.compile(r'Expected to .+, but passed:\s+\((\d+)\)')), > - ('missing results', re.compile(r'(?:no expected results found|missing results)\s*:\s+\((\d+)\)')), > - ('failures', re.compile(r'Regressions: [Uu]nexpected.+:?\s*\((\d+)\)')), > + ('missing results', re.compile(r'Regressions: Unexpected missing results\s+\((\d+)\)')), > + ('failures', re.compile(r'Regressions: Unexpected.+\((\d+)\)')), Is this removing ORWT support?
Comment on attachment 178871 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=178871&action=review >> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:371 >> + ('failures', re.compile(r'Regressions: Unexpected.+\((\d+)\)')), > > Is this removing ORWT support? No, this function is specific to NRWT.
(In reply to comment #8) > (From update of attachment 178871 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=178871&action=review > > > Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:82 > > +Regressions: Unexpected missing results (1) > > + svg/custom/zero-path-square-cap-rendering2.svg [ Missing ] > > We probably shouldn't report missing results as "Regressions" if tests just had been added. Unfortunately, there's no way to tell if the tests had just been added, but this change is actually at least breaking out the missing category from the other failures.
Committed r137404: <http://trac.webkit.org/changeset/137404>
The bots are running w/ this change and seem happy so far, but I haven't seen any "missing" results yet, so I'm not 100% positive it worked. Ossy, can you help me keep on the lookout for this?
Hi Dirk, I stepped over this failure here and I'm suspected that it might be caused by this patch. Can you confirm? $ ./mastercfg_unittest.py /usr/lib/python2.7/dist-packages/twisted/spread/jelly.py:102: DeprecationWarning: the sets module is deprecated import sets as _sets ./mastercfg_unittest.py:819: DeprecationWarning: Passing a BuildStep subclass to factory.addStep is deprecated. Please pass a BuildStep instance instead. Support will be dropped in v0.8.7. ...................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................F............. ====================================================================== FAIL: test_nrwt_leaks_parsing (__main__.MasterCfgTest) ---------------------------------------------------------------------- Traceback (most recent call last): File "./mastercfg_unittest.py", line 64, in test_nrwt_leaks_parsing self.fail() AssertionError: None ---------------------------------------------------------------------- Ran 529 tests in 0.031s FAILED (failures=1)
(In reply to comment #14) > Hi Dirk, > > I stepped over this failure here and I'm suspected that it might be caused by this patch. Can you confirm? > > $ ./mastercfg_unittest.py > /usr/lib/python2.7/dist-packages/twisted/spread/jelly.py:102: DeprecationWarning: the sets module is deprecated > import sets as _sets > ./mastercfg_unittest.py:819: DeprecationWarning: Passing a BuildStep subclass to factory.addStep is deprecated. Please pass a BuildStep instance instead. Support will be dropped in v0.8.7. > ...................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................F............. > ====================================================================== > FAIL: test_nrwt_leaks_parsing (__main__.MasterCfgTest) > ---------------------------------------------------------------------- > Traceback (most recent call last): > File "./mastercfg_unittest.py", line 64, in test_nrwt_leaks_parsing > self.fail() > AssertionError: None > > ---------------------------------------------------------------------- > Ran 529 tests in 0.031s > > FAILED (failures=1) Whoops :). Yeah, my mistake. Will fix shortly.