Bug 36613 - Web Inspector: Cover the Audits panel with tests
Summary: Web Inspector: Cover the Audits panel with tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexander Pavlov (apavlov)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-25 11:27 PDT by Alexander Pavlov (apavlov)
Modified: 2010-03-29 11:01 PDT (History)
7 users (show)

See Also:


Attachments
[PATCH] Test for the Audits panel + drive-by rule fix (8.90 KB, patch)
2010-03-26 04:56 PDT, Alexander Pavlov (apavlov)
pfeldman: review-
Details | Formatted Diff | Diff
[PATCH] Comments addressed (9.19 KB, patch)
2010-03-26 07:15 PDT, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff
[PATCH] Full results tree dump in the test (14.13 KB, patch)
2010-03-26 09:50 PDT, Alexander Pavlov (apavlov)
pfeldman: review+
Details | Formatted Diff | Diff
[PATCH] Comments addressed, some changes for environment-independent testing (18.38 KB, patch)
2010-03-29 08:22 PDT, Alexander Pavlov (apavlov)
pfeldman: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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