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

Sorry for the delayed response.

On 06/04/2013 12:43 PM, Hallvord Reiar M. Steen wrote:
>
> 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 personally don't mind. If there are bugs in the implementation though, 
they might confuse the state handling of the testharness.js in which 
case it's not reliable to continue running subsequent tests.

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

That's a good idea, yes.

Regards,

Dominik

-- 
Dominik Röttsches <dominik.rottsches@intel.com>

Received on Saturday, 8 June 2013 01:26:49 UTC