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