Bug 30478

Summary: Web Inspector: simple console tests do not require testing harness.
Product: WebKit Reporter: Pavel Feldman <pfeldman>
Component: Web Inspector (Deprecated)Assignee: Pavel Feldman <pfeldman>
Status: RESOLVED FIXED    
Severity: Normal CC: bweinstein, timothy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch timothy: review+

Description Pavel Feldman 2009-10-17 09:02:38 PDT
No need to use heavy frontend harness in these tests.
Comment 1 Pavel Feldman 2009-10-17 09:05:05 PDT
Created attachment 41358 [details]
patch
Comment 2 Pavel Feldman 2009-10-17 09:07:05 PDT
(diff is large due to CRLFs in the original one).
Comment 3 Eric Seidel (no email) 2009-10-19 13:44:14 PDT
Comment on attachment 41358 [details]
patch

Do you mean other inspector tests have setTimeout(,500)!?!?!  If so I might explode, that's awful!

These would be even simpler as script-tests.
http://trac.webkit.org/wiki/Writing%20Layout%20Tests%20for%20DumpRenderTree
Comment 4 Pavel Feldman 2009-10-19 15:09:12 PDT
No, but they have setTimeout(,0). That is needed to queue checks... Don't explode - I noticed tests using infrastructure they should not be and made them a bit more lightweight. If you think they can be even lighter - patches are welcome!

Seriously, there are two bad things about inspector testing harness:
1. They require to reload layout test page in the beginning of the test in order to enable inspector controller
2. They do setTimeout(,0) in order to queue stuff and prevent them from reentering code.

Hints on fixing (1) are very welcome.
Comment 5 Timothy Hatcher 2009-10-20 17:19:19 PDT
Comment on attachment 41358 [details]
patch


> +    var p = document.createTextNode();
> +    p.textContent = "foo";
> +    p.splitText(10000);

Why is this needed?
Comment 6 Brian Weinstein 2009-10-20 17:21:34 PDT
(In reply to comment #5)
> (From update of attachment 41358 [details])
> 
> > +    var p = document.createTextNode();
> > +    p.textContent = "foo";
> > +    p.splitText(10000);
> 
> Why is this needed?

It generates an exception, but it could be simpler, this was from my original case, which I took from a case in LayoutTests/dom that generated the same exception.

document.createTextNode("foo").splitText(10000) should work too.
Comment 7 Timothy Hatcher 2009-10-20 17:25:30 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 41358 [details] [details])
> > 
> > > +    var p = document.createTextNode();
> > > +    p.textContent = "foo";
> > > +    p.splitText(10000);
> > 
> > Why is this needed?
> 
> It generates an exception, but it could be simpler, this was from my original
> case, which I took from a case in LayoutTests/dom that generated the same
> exception.
> 
> document.createTextNode("foo").splitText(10000) should work too.

I see. A comment about what it does (throws an exception) would be good.
Comment 8 Brian Weinstein 2009-10-20 17:31:22 PDT
(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > (From update of attachment 41358 [details] [details] [details])
> > > 
> > > > +    var p = document.createTextNode();
> > > > +    p.textContent = "foo";
> > > > +    p.splitText(10000);
> > > 
> > > Why is this needed?
> > 
> > It generates an exception, but it could be simpler, this was from my original
> > case, which I took from a case in LayoutTests/dom that generated the same
> > exception.
> > 
> > document.createTextNode("foo").splitText(10000) should work too.
> 
> I see. A comment about what it does (throws an exception) would be good.

The body of the test says - 

This tests that we are getting the correct message for DOM Exception 1: INDEX_SIZE_ERR.

A comment could help though, Pavel will read this before he commits I'm sure.
Comment 9 Pavel Feldman 2009-10-20 20:38:50 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	M	LayoutTests/inspector/uncaught-dom1-exception-expected.txt
	M	LayoutTests/inspector/uncaught-dom1-exception.html
	M	LayoutTests/inspector/uncaught-dom3-exception-expected.txt
	M	LayoutTests/inspector/uncaught-dom3-exception.html
	M	LayoutTests/inspector/uncaught-dom8-exception-expected.txt
	M	LayoutTests/inspector/uncaught-dom8-exception.html
Committed r49897


(Comment added as requested in the comments).