Bug 101469 – REGRESSION(r133380): new tests without expected file reported as failing tests on the bots

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Csaba Osztrogonác]

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.

[Dirk Pranke]

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.

[Csaba Osztrogonác]

Are you going to fix this annoying regression?

[Dirk Pranke]

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.

[Dirk Pranke]

Okay, I thought this had been fixed ... were the changes in bug 93537 and bug 93789 not sufficient? What's still broken?

[Csaba Osztrogonác]

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

[Csaba Osztrogonác]

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

[Dirk Pranke]

Created attachment 178871 [details]
Patch

[Ryosuke Niwa]

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.

[Eric Seidel (no email)]

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?

[Ryosuke Niwa]

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.

[Dirk Pranke]

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

[Dirk Pranke]

Committed r137404: <http://trac.webkit.org/changeset/137404>

[Dirk Pranke]

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?

[Thiago Marcos P. Santos]

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)

[Dirk Pranke]

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