Bug 36613

Summary: Web Inspector: Cover the Audits panel with tests
Product: WebKit Reporter: Alexander Pavlov (apavlov) <apavlov>
Component: Web Inspector (Deprecated)Assignee: Alexander Pavlov (apavlov) <apavlov>
Status: RESOLVED FIXED    
Severity: Normal CC: bweinstein, joepeck, keishi, pfeldman, pmuellr, rik, timothy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Test for the Audits panel + drive-by rule fix
pfeldman: review-
[PATCH] Comments addressed
none
[PATCH] Full results tree dump in the test
pfeldman: review+
[PATCH] Comments addressed, some changes for environment-independent testing pfeldman: review+

Description Alexander Pavlov (apavlov) 2010-03-25 11:27:11 PDT
Currently there are no tests for the Audits panel, which should be fixed.
Comment 1 Alexander Pavlov (apavlov) 2010-03-26 04:56:10 PDT
Created attachment 51727 [details]
[PATCH] Test for the Audits panel + drive-by rule fix
Comment 2 Pavel Feldman 2010-03-26 05:30:09 PDT
Comment on attachment 51727 [details]
[PATCH] Test for the Audits panel + drive-by rule fix

> +<!-- These scripts are needed to result in a violation of the max JS file count from the same domain -->

You should put artificial scripts into the resources folder for this test instead.
 
> +            frontend_assertEquals("'" + ruleResult.value + "' violation count", expectedData[ruleResult.value], ruleResult.violationCount);

You should push information back to the inspected page instead and dump it onto the screen. That way expectations would reflect what is happening in the test.
Comment 3 Alexander Pavlov (apavlov) 2010-03-26 07:15:40 PDT
Created attachment 51739 [details]
[PATCH] Comments addressed
Comment 4 Alexander Pavlov (apavlov) 2010-03-26 09:50:39 PDT
Created attachment 51754 [details]
[PATCH] Full results tree dump in the test
Comment 5 Pavel Feldman 2010-03-28 05:15:39 PDT
Comment on attachment 51754 [details]
[PATCH] Full results tree dump in the test

Couple of nits you should fix prior to landing:
- WebInspector.displayNameForURL is now test-friendly, no need to work around it
- You should try adding line breaks into the results dump as I do it for styles and elements tree for example.
Comment 6 Alexander Pavlov (apavlov) 2010-03-29 08:22:19 PDT
Created attachment 51912 [details]
[PATCH] Comments addressed, some changes for environment-independent testing

Apologies for another round, I spotted another host-dependent test result (resource coalescing audit which involved printing resource.domain), so had to add an interim method to change one in test.
Comment 7 Alexander Pavlov (apavlov) 2010-03-29 11:01:01 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
        M       LayoutTests/ChangeLog
        M       WebCore/ChangeLog
        M       WebCore/English.lproj/localizedStrings.js
        M       WebCore/inspector/front-end/AuditResultView.js
        M       WebCore/inspector/front-end/AuditRules.js
        M       WebCore/inspector/front-end/AuditsPanel.js
        M       WebCore/inspector/front-end/inspector.js
Committed r56732