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>

Status:

RESOLVED FIXED

Severity:

Normal

Priority:

Version:

528+ (Nightly build)

Hardware:

Mac

OS:

OS X 10.5

Attachments:

Description	Flags
The moving and renaming of the manual files	timothy: review+
Actual changes to the tests	timothy: review+
Expected results	none
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.