Bug 138898 – Web Inspector: LayoutTests/inspector tests fail in Production builds due to missing test resources

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

* SUMMARY
Web Inspector: LayoutTests/inspector tests fail in Production builds due to missing test resources (Test.html/Tests.js)

* NOTES
- We should provide a way to still build Production and include these test resources.
- Mac already does this in some places with the configuration variable FORCE_TOOL_INSTALL

[Joseph Pecoraro]

<rdar://problem/16200275>

[Joseph Pecoraro]

Created attachment 241909 [details]
[PATCH] Proposed Fix

[Alexey Proskuryakov]

Comment on attachment 241909 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=241909&action=review

> LayoutTests/platform/mac/TestExpectations:1419
>  [ Yosemite+ ] inspector/css/matched-style-properties.html [ Pass Failure Crash ]

You could probably remove at least the Crash part, and maybe change skipped tests into "Pass Failure".

I wonder what is special about Yosemite that makes these tests flaky.

[Joseph Pecoraro]

> You could probably remove at least the Crash part, and maybe change skipped
> tests into "Pass Failure".

Ahh, yes I can do that.

> I wonder what is special about Yosemite that makes these tests flaky.

Yeah, the flakiness is a big mystery which will take some investigation.

[Joseph Pecoraro]

Created attachment 242322 [details]
[PATCH] Proposed Fix

Updated to remove allowing [Crash] on inspector tests. The flakiness has another bug, but crashing should be fixed.

There is a different test that seems like it might ASSERT on debug builds, so I left the Crash for that (in a separate section of TestExpectations).

[Mark Rowe (bdash)]

Comment on attachment 242322 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=242322&action=review

> Source/WebInspectorUI/Scripts/copy-user-interface-resources-dryrun.rb:34
> +  # Further debugging.
> +  # ENV["FORCE_TOOL_INSTALL"] = "YES"
> +

I'm not clear why this is something that should be checked in.

[Joseph Pecoraro]

Comment on attachment 242322 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=242322&action=review

> Source/WebInspectorUI/ChangeLog:8
> +        In Production builds, if FORCE_TOOL_INSTALL=YES in in the environment

Typo: "in in" => "is in"

>> Source/WebInspectorUI/Scripts/copy-user-interface-resources-dryrun.rb:34
>> +
> 
> I'm not clear why this is something that should be checked in.

I'll remove it before landing. Alexey also questioned this.

[Joseph Pecoraro]

http://trac.webkit.org/changeset/176815