- From: James Graham <jgraham@opera.com>
- Date: Fri, 03 May 2013 09:43:09 +0200
- To: "Zhao, Tina" <tina.zhao@intel.com>
- CC: "public-webapps-testsuite@w3.org" <public-webapps-testsuite@w3.org>
On 05/03/2013 04:10 AM, Zhao, Tina wrote:
>
> -----Original Message-----
> From: James Graham [mailto:jgraham@opera.com]
> Sent: Thursday, May 02, 2013 8:22 PM
> To: public-webapps-testsuite@w3.org
> Subject: Re: Fwd: ACTION-681: Ask Tina to remove the IE column from the SSE implementation report (Web Applications Working Group)
>
> On 05/02/2013 02:01 PM, Zhao, Tina wrote:
>
>> 7. format-field-retry-bogus.htm, format-field-retry.htm: default
>> duration (3000) of test harness timeout is too short to check the
>> retry function, so set the timeout to 10000. Test case bug. Patch
>> attached.
>
> Can you make this into a pull request? If you do I will review it for you :)
> [Tina] https://github.com/Honry/event-source/commit/7e56d66560f5046cab5bea0581d0a39c889ed2c4
Right, you didn't quite go the final step and actually turn this into a
pull request, as far as I can see. If you don't do this it's hard to
review and merge.
Since it is only a small change, a couple of review comments by mail:
If you increase the timeout of the test to 10000ms you need to increase
teh timeout for the entire file to something larger (or you might get a
"not run" status, even though the test started). You should do this
using something like
setup({timeout:10100})
Also, these days we have assert_less_than and friends, so it isn't
necessary to do things like assert_true(a < b). It would be nice to also
fix those parts of the tests.
Received on Friday, 3 May 2013 07:43:42 UTC