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

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



My first impression is that these tests are written in a somewhat complicated way - I understand they are adapted from somewhere else, but it would be good both for test stability, for implementors debugging bugs exposed by the tests, and for those who might want to extend the test suite and add more tests if they were simpler.



There is a specific framework dedicated to these tests (xmlhttprequest-timeout.js), all HTML pages contain exactly the same script which calls a method in the .js file that looks at the page's file name (!) to figure out what tests to run. The script then uses postMessage() to kick of the actual tests. This is more or less to simplify running the same tests in a Worker thread and a page, but it takes a bit of time to find the code that actually executes the test. I'd prefer if the code that matters for the test was in the file that "is" the test itself. (In this case that might mean some more code duplication, so I guess it's a YMMV thing.) I assume much of the same effect could be achieved by putting the essential code for each test in a .js file and linking it to the main file either with a SCRIPT src=.. or a Worker.


Ideally, if the submitters have time to somewhat simplify and refactor the tests I think it would be a good idea. Otherwise, I'll move ahead and review them further. I'd like to run some "test stress tests" (i.e. run each of the tests some 500 - 1000 times in a number of browsers) to make sure that they return consistent results even though they are a little more complex than they could be.


PS AbortedRequest.prototype.startXHR() method doesn't need both a "me" and a "_this" variable.-- 
Hallvord R. M. Steen
Core tester, Opera Software

Received on Friday, 3 May 2013 13:20:25 UTC