Bug 21817

Summary: Manual profiler tests should be made into layout tests
Product: WebKit Reporter: Kevin McCullough <kmccullough>
Component: Web Inspector (Deprecated)Assignee: Kevin McCullough <kmccullough>
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Description Flags
The moving and renaming of the manual files
timothy: review+
Actual changes to the tests
timothy: review+
Expected results
Expected results timothy: review+

Description Kevin McCullough 2008-10-22 16:51:06 PDT
There are many manual tests that  are in the WebCore/manual-tests/inspector directory that should be moved into the layout tests directory now that we can interact with profiles via JS.
Comment 1 Kevin McCullough 2008-10-23 10:12:56 PDT
Created attachment 24601 [details]
The moving and renaming of the manual files

The changes to the files and expected output will come next.
Comment 2 Timothy Hatcher 2008-10-23 11:20:17 PDT
Comment on attachment 24601 [details]
The moving and renaming of the manual files

How do you plan to make the expected results meaningful? I think a better first step would be to create a shared function that outputs the profile data in a text representation that shows the tree structure. As-is just moving these tests to the LayoutTests directory does nothing useful.
Comment 3 Kevin McCullough 2008-10-23 11:34:16 PDT
Committed revision 37816.  Which moved all the files, now I need to change them.
Comment 4 Kevin McCullough 2008-10-23 11:44:15 PDT
Committed revision 37817. Forgot to actually do the move :(
Comment 5 Kevin McCullough 2008-10-23 11:48:56 PDT
Created attachment 24606 [details]
Actual changes to the tests

Changes but no expected results files.  Those are next.
Comment 6 Timothy Hatcher 2008-10-23 13:10:50 PDT
Comment on attachment 24606 [details]
Actual changes to the tests

+        - Here are the changes to the tests.  The most significant part is the
+        change to profiler-test-JS-resources.js

Not a good ChangeLog entry. Describe the changes/additions.

+    ++tab;

A better name for tab would be indentLevel.

+    for (var j = 0; j < children.length; ++j)

No need to use j here, you can use i again.

r=me, but you should consider some of these changes.
Comment 7 Kevin McCullough 2008-10-23 13:29:32 PDT
Committed revision 37822.  Which checks in the changes to the tests.  Now all that remains is the expected results.
Comment 8 Kevin McCullough 2008-10-23 13:53:48 PDT
Created attachment 24614 [details]
Expected results

Final piece.  The expected results.
Comment 9 Timothy Hatcher 2008-10-23 14:13:07 PDT
Comment on attachment 24614 [details]
Expected results

Maybe you should remove/revise the line of text in many tests:

"To use this test, load it in the browser then load the WebInspector and look at the profile."
Comment 10 Kevin McCullough 2008-10-23 15:11:07 PDT
Committed revision 37829. Which should be the last of it until new tests are added.