[Webkit-unassigned] [Bug 35350] DumpRenderTree should allow tests with modal dialogs

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=35350





--- Comment #18 from Prasad Tammana <prasadt at chromium.org>  2010-06-18 15:48:15 PST ---
(In reply to comment #17)
> (From update of attachment 58829 [details])
> > +#if PLATFORM(MAC)
> 
> While the need for this may be specific to the Mac, I think the concept can exist cross platform. We could use a less specific name for the function, but I suggest we have an empty function on other platforms rather than not function at all. In the future it could make it easier to write tests. We don’t want a long term strategy that adds something that’s only for one platform.

That makes sense.  I didn't realize that I'm making the test Mac specific by doing this.  I'll do another patch to add an empty function and make this platform independent once this patch goes through - https://bugs.webkit.org/show_bug.cgi?id=40864.  And later when I implement this for other platforms, I'll rename/re-factor as appropriate.

> I’m going to say it’s OK to land this test as-is, but I don’t completely agree with every aspect of the approach here. Disabling tests that are expected to fail is not the best way to deal with them, and there’s a lot of test disabling here. I also don’t like patches with promises for the future in them.

The other way to handle that I considered and initially implemented was to create a platform specific expected output for MAC.  That would've obviated the need to skip tests but comes with two downsides:
1) The patch was 350k making it huge.
2) It'd be cumbersome and error prone to keep the mac specific expected output files in sync with the rest as some of these files are huge.

> I don’t understand what the 10.2.2 tests have to do with showModalDialog.

The theme of these tests is - "scope chain must contain same objects in the same order as the calling context".  The test concatenates all the properties on the window object in the current context and in eval context and compares the strings.  showModalDialog being undefined seems to cause it to end up at different places in the sort order in different contexts.  And so all these tests fail.  Implementing showModalDialog method caused the tests to start passing.  One could say that these were (effectively) disabled on all platforms and now enabled on Mac.

I could've fixed these tests to ignore showModalDialog in creating this concatenated string and have them pass on all platforms.  As I understand, these are imported from somewhere and not supposed to be modified.

As an aside, these are terribly written tests.  Try this:
LayoutTests/fast/js/sputnik/Conformance/10_Execution_Contexts/10.2_Entering_An_Execution_Context/10.2.2_Eval_Code/
tkdiff S10.2.2_A1.1_T3.html S10.2.2_A1.1_T7.html

You'll see a couple lines of differences between the two files each of which is about 100+ lines and there are couple dozen of these files.  Can't imagine there not being a better way to do this.

Thanks for the review.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.