----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16099/#review30038 -----------------------------------------------------------
e2e/hooker.spec.js <https://reviews.apache.org/r/16099/#comment57545> For non-win platforms there is logic to skip bat files (with a warning). But looks like on windows .sh files will be executed. The old version was just copying the right files from the fixtures dir as part of the test. Moving the extra complexity out from the code into the folder structure seemed more elegant to me (though that's entirely a matter of taste). e2e/hooker.spec.js <https://reviews.apache.org/r/16099/#comment57546> The hooker does have logic to execute the handlers serially, wrapping them in Q promises. It's here: https://github.com/apache/cordova-cli/blob/master/src/hooker.js#L142 But this test wasn't really checking it, I've changed h1 to return a promise that resolves after a delay of 0.1 second. e2e/hooker.spec.js <https://reviews.apache.org/r/16099/#comment57547> That's right, but this is an error handler passed to Q.then, I don't think it can ever be called with no error argument. But I changed this to a different pattern that looks like this: fire(test_handler) .then(expectations) .fail(err_handler) .fin(done) This one is better if an exception is raised by some code in the expectations checking function. In the previous case it's that exception would be silented by the Q lib + jasmine because Q has no error handler to pass it to and jasmine ignores the re-raised exception as long as done() is called by the .fin(). e2e/hooker.spec.js <https://reviews.apache.org/r/16099/#comment57548> The credit is not mine, was there before :) e2e/hooker.spec.js <https://reviews.apache.org/r/16099/#comment57549> The credit is not mine, was there before :) - Mark Koudritsky On Dec. 9, 2013, 10:19 p.m., Mark Koudritsky wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16099/ > ----------------------------------------------------------- > > (Updated Dec. 9, 2013, 10:19 p.m.) > > > Review request for cordova. > > > Bugs: CB-4382 and CB-5330 > https://issues.apache.org/jira/browse/CB-4382 > https://issues.apache.org/jira/browse/CB-5330 > > > Repository: cordova-cli > > > Description > ------- > > Note: Two commits in diff (created as git format-patch). Same diff on github: > https://github.com/kamrik/cordova-cli/compare/hook_vars > > - Refactored the hooker.spec.js to use real files like the e2e tests. > - Moved the spec and corresponding fixtures to live under e2e dir. > - Rearranged the hooks fixtures into two separate dirs for Win and non-Win > platforms. > - e2e tests on windows can interfere with one another (seem to run partially > in parallel), changed them to use different tmp subdirs for each test. > > > Diffs > ----- > > e2e/create.spec.js 3f1304c > e2e/fixtures/hooks_bat/fail/fail.bat PRE-CREATION > e2e/fixtures/hooks_bat/test/.dotted.bat PRE-CREATION > e2e/fixtures/hooks_bat/test/07.bat PRE-CREATION > e2e/fixtures/hooks_bat/test/1.bat PRE-CREATION > e2e/fixtures/hooks_sh/fail/fail.sh PRE-CREATION > e2e/fixtures/hooks_sh/test/.dotted.sh PRE-CREATION > e2e/fixtures/hooks_sh/test/07.sh PRE-CREATION > e2e/fixtures/hooks_sh/test/1.sh PRE-CREATION > e2e/helpers.js aa1c790 > e2e/hooker.spec.js PRE-CREATION > e2e/platform.spec.js be5761e > e2e/plugin.spec.js dd493bb > package.json 6c1c753 > spec/fixtures/hooks/fail/fail.bat 0c810b7 > spec/fixtures/hooks/fail/fail.sh 379a4c9 > spec/fixtures/hooks/test/07.bat 1095fc0 > spec/fixtures/hooks/test/07.sh 6e25461 > spec/fixtures/hooks/test/1.bat 4e76af0 > spec/fixtures/hooks/test/1.sh 53d5e97 > spec/hooker.spec.js d4b073e > src/hooker.js 06acec7 > > Diff: https://reviews.apache.org/r/16099/diff/ > > > Testing > ------- > > npm test > cordova prepare with dummy prepare hooks that store all environment variables. > > > Thanks, > > Mark Koudritsky > >
