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+

Kevin McCullough

Reported 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.

Attachments
The moving and renaming of the manual files (deleted) 2008-10-23 10:12 PDT, Kevin McCullough	timothy: review+	Details Formatted Diff Diff
Actual changes to the tests (deleted) 2008-10-23 11:48 PDT, Kevin McCullough	timothy: review+	Details Formatted Diff Diff
Expected results (deleted) 2008-10-23 13:51 PDT, Kevin McCullough	no flags	Details Formatted Diff Diff
Expected results (70.21 KB, patch) 2008-10-23 13:53 PDT, Kevin McCullough	timothy: review+	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Kevin McCullough

Comment 1 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.

Timothy Hatcher

Comment 2 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.

Kevin McCullough

Comment 3 2008-10-23 11:34:16 PDT

Committed revision 37816. Which moved all the files, now I need to change them.

Kevin McCullough

Comment 4 2008-10-23 11:44:15 PDT

Committed revision 37817. Forgot to actually do the move :(

Kevin McCullough

Comment 5 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.

Timothy Hatcher

Comment 6 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.

Kevin McCullough

Comment 7 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.

Kevin McCullough

Comment 8 2008-10-23 13:53:48 PDT

Created attachment 24614 [details] Expected results Final piece. The expected results.

Timothy Hatcher

Comment 9 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."

Kevin McCullough

Comment 10 2008-10-23 15:11:07 PDT

Committed revision 37829. Which should be the last of it until new tests are added.

Note You need to log in before you can comment on or make changes to this bug.