Bug 138898 - Web Inspector: LayoutTests/inspector tests fail in Production builds due to missing test resources
Summary: Web Inspector: LayoutTests/inspector tests fail in Production builds due to m...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: DoNotImportToRadar
Depends on:
Blocks:
 
Reported: 2014-11-19 16:46 PST by Joseph Pecoraro
Modified: 2014-12-04 13:55 PST (History)
6 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (4.34 KB, patch)
2014-11-19 16:59 PST, Joseph Pecoraro
joepeck: review-
joepeck: commit-queue-
Details | Formatted Diff | Diff
[PATCH] Proposed Fix (5.47 KB, patch)
2014-12-01 11:25 PST, Joseph Pecoraro
mrowe: review+
mrowe: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2014-11-19 16:46:55 PST
* 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
Comment 1 Joseph Pecoraro 2014-11-19 16:47:11 PST
<rdar://problem/16200275>
Comment 2 Joseph Pecoraro 2014-11-19 16:59:47 PST
Created attachment 241909 [details]
[PATCH] Proposed Fix
Comment 3 Alexey Proskuryakov 2014-11-19 17:07:27 PST
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.
Comment 4 Joseph Pecoraro 2014-11-19 17:16:50 PST
> 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.
Comment 5 Joseph Pecoraro 2014-12-01 11:25:36 PST
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).
Comment 6 Mark Rowe (bdash) 2014-12-04 12:34:59 PST
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.
Comment 7 Joseph Pecoraro 2014-12-04 12:54:29 PST
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.
Comment 8 Joseph Pecoraro 2014-12-04 13:55:58 PST
http://trac.webkit.org/changeset/176815