Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix how the promises are created and handle them properly in the tests #4

Merged

Conversation

Elgolfin
Copy link
Contributor

  • Catch the possible errors from the find library (find.file method)
    • With the built-in error handler for the async method
    • With a try/catch for the sync method
    • Properly reject the generated promise with the given error (if any)
  • Fix the unit tests to properly handle the promises following the below guidelines
    https://www.sitepoint.com/promises-in-javascript-unit-tests-the-definitive-guide/
    • Add Chai As Promised to do so
    • Remove all done() callbacks (useless)
    • Add a unit test to handle an error when the root directory does not exist

… tests

  - Catch the possible errors from the find library (find.file method)
    - With the built-in error handler for the async method
    - With a try/catch for the sync method
    - Properly reject the generated promise with the given error (if any)
  - Fix the unit tests to properly handle the promises following the below guidelines
    https://www.sitepoint.com/promises-in-javascript-unit-tests-the-definitive-guide/
    - Add Chai As Promised to do so
    - Remove all done() callbacks (useless)
    - Add a unit test to handle an error when the root directory does not exist
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 460a25e on Elgolfin:pull-request-promises-management into 49efda7 on kaesetoast:master.

@kaesetoast kaesetoast merged commit 8257acd into kaesetoast:master Mar 20, 2017
@kaesetoast
Copy link
Owner

@Elgolfin thanks a lot! :-)

@Elgolfin
Copy link
Contributor Author

You are very welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants