Re: [Fwd: Re: Re: Request for review of XHR test Pull Request #91 [Was: Re: Intel submission: XHR timeout tests]]

On Fri, 17 May 2013 15:28:15 +0200, Dominik Röttsches  
<dominik.rottsches@intel.com> wrote:

> as you might have seen, I updated the pull request with your comments  
> hopefully satisfactorily addressed.
> https://github.com/w3c/web-platform-tests/pull/91

Hi Dominik,
I've merged the tests in but - just FYI - I plan to make some changes to  
them.

One issue I'm wondering about is why
http://w3c-test.org/web-platform-tests/submissions/91/XMLHttpRequest/xmlhttprequest-timeout-runner.js

contains this code:

// Abort test execution if an individual test case fails.
add_result_callback(function (t) {
if (t.status == t.FAIL)
done();
});

This terminates running the tests on the first failure. I'd much rather  
see ALL the sub-tests listed on the results page than seeing only the  
sub-tests up to the first failure..

This could also mask regressions - if you have a set of 3 tests, #2 fails  
but #3 would pass until a certain change. After that change both #2 and #3  
fail but you won't learn about the regression from running the test suite  
if the #2 failure terminates testing.

Do you mind if I remove this?

I would also like to see the more detailed test descriptions in the test  
results list. I suggest changing

test(function() { assert_equals(event.data.got, event.data.expected,  
event.data.msg); });

into

test(function() { assert_equals(event.data.got, event.data.expected); } ,  
event.data.msg );

(passing the message to test() as the description of the test, rather than  
passing it in as the description of the failure only). It makes it a lot  
clearer what is being tested.

I plan to check in these changes with pull request 128.
-Hallvord