Microsoft IndexedDB test submissions review

I've gone through some of the submitted tests for IndexedDB by Microsoft,  
and found some issues, here they are sorted by severeness:

Issue 1
-------

This is a known issue, as it's already raised by Israel Hilerio from  
Microsoft:

   The tests are using the old IDBFactory.open() API.


Issue 2
-------

The tests are abstracting away all error handling (to PassTest() and  
FailTest()) instead of using the testharness primitives directly. This  
means the only error message we will ever get is "assert_true: Expected  
true got false" which IMHO isn't very helpful.


As far as I can see, there are two main patterns here, first one:

     try {
         <some code>
     } except {
        FailTest();
     }

I propose instead to do:

     t.step_func(function() {
         <some code>
     }

As that will print the exception in the message field, allowing us to see  
relevant information about the exception.


The other pattern:

     rqGet.onsuccess = function(event)
     {
         if (record.property == event.target.result.property)
         {
             PassTest();
         }
         else
         {
             FailTest();
         }
     };

I propose instead to do:

     rqGet.onsuccess = t.step_func(function(event)
     {
         assert_equals(event.target.result.property, record.property,  
'event.target.result.property');
     })


A last pattern (but I said two, but anyway) is:

     db.onerror = TestFail;

which is shorter than the alternative, but it doesn't say what is failing,  
so I propose something like:

     db.onerror = t.step_func(function(e) { assert_unreached('Database  
error: ' + e.message) });


Much of the code will be shorter as an benefit of the first two, even if  
we get a few extra t.step() and t.step_func() scattered around.


PassTest() and FailTest() also has another function, that is closing the  
database. This can be done in the "complete" callback of testharness.js:

     add_completion_callback(function() { if(db) db.close(); })


Issue 3
-------

The path is hardcoded to w3c-test.org, the tests would be more portable  
(e.g. to my laptop or to a local test framework) if it didn't use absolute  
URLs. So instead of:

         <script  
src="http://w3c-test.org/resources/testharness.js"></script>
         <script  
src="http://w3c-test.org/resources/testharnessreport.js"></script>

Rather:
         <script src="/resources/testharness.js"></script>
         <script src="/resources/testharnessreport.js"></script>

Especially testharnessreport is supposed to be overwritten by your custom  
report code reporting results to whatever system you want, but that won't  
be doable if it reads w3c's testharnessreport instead of your local  
version.


Issue 4
-------

The Initialize() function could maybe be run from the setup() function.


Issue 5
-------

No need to do   var t = async_test(document.title)   as   var t =  
async_test()   is equivalent.


Issue 6
-------

Apache has a bug where it somehow only reads <title> tags and uses that as  
descriptions in automatic folder listings if it doesn't have any  
attributes. That feature isn't turned on in w3c-test.org, but it would be  
very helpful if it were, because the descriptions are good in the tests,  
but the filenames doesn't give as much information.

Anyway, if w3c-test.org DO turn that Apache feature on, it will still not  
work with these tests unless you remove the id='desc' in the title.



-- 
Odin Hørthe Omdal · Core QA, Opera Software · http://opera.com /

Received on Monday, 9 January 2012 17:12:45 UTC