Bug 196016 – webkit-patch upload doesn't run 'check-style-local' so doesn't find TestExpectations errors

• Status: NEW
• Component: Tools / Tests

[Simon Fraser (smfr)]

I just uploaded a patch where a file added to TestExpectations didn't exist (https://bugs.webkit.org/attachment.cgi?id=365355&action=prettypatch), but webkit-patch didn't warn me.

Style checker runs 'check-style-local' but webkit-patch doesn't do that?

[Simon Fraser (smfr)]

lint-test-expectations seems really broken; we should remove it.

[Jonathan Bedard]

I'm a bit confused.

You're saying that EWS caught it but webkit-patch didn't?

[Jonathan Bedard]

Looking a bit at the code and doing some local testing:

lint-test-expectations is weird because we have a whole bunch of defunct buildbot stuff sitting around in our code which defines ports automation is using. This stuff gets out of date super fast, and we shouldn't rely on it (lint-test-expectations does rely on it)

What's more, lint-test-expectations does really weird things with port object when you try and define it (via --ios-simulator, for example)

With all that being said, the core logic of lint-test-expectations is not the problem. Its port management is. I think looking at Simon's patch, 'lint-test-expectations' with no arguments might have actually flagged what he wanted it to.

[Simon Fraser (smfr)]

(In reply to Jonathan Bedard from comment #2)
> I'm a bit confused.
> 
> You're saying that EWS caught it but webkit-patch didn't?

Yes

[Simon Fraser (smfr)]

'lint-test-expectations' with no arguments told me about Windows expectations.

[Jonathan Bedard]

(In reply to Simon Fraser (smfr) from comment #5)
> 'lint-test-expectations' with no arguments told me about Windows
> expectations.

That's what it should do (along with the iOS ones). lint-test-expectations basically attempts to lint expectations for multiple ports.

[Alexey Proskuryakov]

lint-test-expectations doesn't produce the same result as "run-webkit-tests --lint". I have actually never heard of the former, why do we have two ways to lint?

But the difference between two tools seems at best tangentially related to the issue at hand. webkit-patch is supposed to check style by default AFAIK, so it seems surprising that it finds fewer issues than the style EWS.

Funny that we also have bug 102009 which claims that webkit-patch ALWAYS complains about TestExpectations.