Bug 30478 – Web Inspector: simple console tests do not require testing harness.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Pavel Feldman]

No need to use heavy frontend harness in these tests.

[Pavel Feldman]

Created attachment 41358 [details]
patch

[Pavel Feldman]

(diff is large due to CRLFs in the original one).

[Eric Seidel (no email)]

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

[Pavel Feldman]

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.

[Timothy Hatcher]

Comment on attachment 41358 [details]
patch


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

Why is this needed?

[Brian Weinstein]

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

[Timothy Hatcher]

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

[Brian Weinstein]

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

[Pavel Feldman]

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