Antranig Basman antranig.basman at colorado.edu
Thu Sep 22 10:22:32 UTC 2011

I had a look over the code for this pull request, and it looks good by eye - however, I can't test it since 
currently the only way to demonstrate the issue is to build and run the PHP server. This highlights a gap in 
our test case coverage - in fact, there are currently no unit tests at all which test file error conditions 
in the uploader, or even that a listener registered for errors can fire.
Mlam - could you add some material to this pull request to add some unit tests which can verify that the fix 
works? A good place for them looks like the "checkUploaderIntegration" function in UploaderTests.js line 
355. There are already a lot of checks for the proper condition of the uploader button, and we can add in 
one here to deal with the case where "all files uploaded are in error".

You should upgrade the XHR mock at line 273 so that it is stateful, and will fire an error whan asked to 
upload specially marked files (you can insert extra args into the demands block at line 233, and add to the 
collection of "files" at line 160). There should also be an onFileError listener added to the overall 
component in the test which checks that this event is fired.


More information about the fluid-work mailing list