Bug 36666

Summary: Newly added test open-database-over-quota.html fails for chromium
Product: WebKit Reporter: David Levin <levin>
Component: DOMAssignee: Eric U. <ericu>
Status: RESOLVED WONTFIX    
Severity: Major CC: darin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch levin: review-

Description David Levin 2010-03-26 10:17:49 PDT
Run the layout test for chromium and you get 
  This tests that calling openDatabase with a size over 5MB doesn't assert on debug builds.
  FAIL: We shouldn't have been able to open the database.
Comment 1 Eric U. 2010-03-26 10:49:09 PDT
Created attachment 51759 [details]
Patch
Comment 2 Darin Adler 2010-03-26 10:55:33 PDT
Comment on attachment 51759 [details]
Patch

So now this will test less. Good for cross-platform portability of the test, but not as good for test coverage.

Shouldn't we have a test that actually checks the behavior of over-quota opening? Even if the results are currently different for different platforms.
Comment 3 Eric U. 2010-03-26 11:11:54 PDT
The point of this test was to verify that a particular crash was fixed.

The behavior of openDatabase with a too-big requested size isn't actually defined in http://dev.w3.org/html5/webdatabase/; either returning a database or throwing an exception is a reasonable response.  I don't think we need to constrain that further with a test.
Comment 4 Eric U. 2010-03-26 11:35:50 PDT
Created attachment 51766 [details]
Patch
Comment 5 Darin Adler 2010-03-26 12:50:39 PDT
(In reply to comment #3)
> The behavior of openDatabase with a too-big requested size isn't actually
> defined in http://dev.w3.org/html5/webdatabase/; either returning a database or
> throwing an exception is a reasonable response.  I don't think we need to
> constrain that further with a test.

There may be a misunderstanding here about the purpose of our tests.

A test doesn't "constrain" -- it exercises code and demonstrates that it still behaves the same as when the test was originally checked in.

Tests should not be limited to things specified in the standards. Our goal is to exercise our code and make sure it's doing what we think it is.
Comment 6 Eric U. 2010-03-26 12:55:22 PDT
OK; that particular part of the behavior wasn't actually why I wrote this test, though.  Still, it could be added, either here or in another test specifically dedicated to testing that part.

Given that Chromium and the rest of WebKit behave differently on openDatabase, what's the right thing to do?  Write a test that behaves differently on one platform, but add it to the test expectations?  Test the behavior, but accept either result?  Other?
Comment 7 Darin Adler 2010-03-26 13:00:39 PDT
(In reply to comment #6)
> Given that Chromium and the rest of WebKit behave differently on openDatabase,
> what's the right thing to do?  Write a test that behaves differently on one
> platform, but add it to the test expectations?  Test the behavior, but accept
> either result?  Other?

The normal way to do it is to have one test, and multiple sets of expected results.
Comment 8 Eric U. 2010-03-29 13:46:56 PDT
(In reply to comment #7)
> (In reply to comment #6)
> > Given that Chromium and the rest of WebKit behave differently on openDatabase,
> > what's the right thing to do?  Write a test that behaves differently on one
> > platform, but add it to the test expectations?  Test the behavior, but accept
> > either result?  Other?
> 
> The normal way to do it is to have one test, and multiple sets of expected
> results.

That's the current state of this test, then?
If there's a prettier way to do it, just point me at an example and I'll switch it over.  If not, I'll close this bug as "as intended".
Comment 9 Eric U. 2010-03-29 14:16:56 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > Given that Chromium and the rest of WebKit behave differently on openDatabase,
> > > what's the right thing to do?  Write a test that behaves differently on one
> > > platform, but add it to the test expectations?  Test the behavior, but accept
> > > either result?  Other?
> > 
> > The normal way to do it is to have one test, and multiple sets of expected
> > results.
> 
> That's the current state of this test, then?
> If there's a prettier way to do it, just point me at an example and I'll switch
> it over.  If not, I'll close this bug as "as intended".

David's pointed me to how it's done.  I'll upload a better fix.
Comment 10 David Levin 2010-03-29 14:36:29 PDT
Comment on attachment 51766 [details]
Patch

It sounds like we're going with a different way of addressing the issue.