search

tj / node-growl

growl unobtrusive notification system for nodejs
481 stars 64 forks source link

Add unit tests for javascript code #71

Open hmshwt opened 6 years ago

hmshwt commented 6 years ago

Issue: There are no unit tests for JS code and running the current tests requires the developer to manually check if all notifications are displayed or not.

Proposed Solution:

I would like to open a pull request for this issue. Please suggest another testing framework or any other libraries that you think will be a better alternative than the choices listed above.

deiga commented 6 years ago

Mocha & Chai seem like a good, stable choice. If you want to make a PR about implementing the basic structures that would be lovely :)

hmshwt commented 6 years ago

Great! I was on vacation, I will open a PR by end of this week.

hmshwt commented 6 years ago

Hi @deiga I am thinking of approaching in the following manner

  1. Extract https://github.com/tj/node-growl/blob/master/lib/growl.js#L304-L334 into a different function and make the function growl() return cmdToExec
  2. Write a new set of integration tests based on the https://github.com/tj/node-growl/blob/master/test.js using the BATS framework.
  3. Using the above BATS test as regression tests, refactor the growl.js and extract all platform specific code into three separate modules (darwin, linux, windows)
  4. Write unit tests for the three platform specific modules
  5. Write another set of unit tests for whatever code remains in the main growl.js module

I believe such a refactor will help us to make the current code easier to test and make it more maintainable in future iterations with platform specific code going into their separate modules.

What do you suggest?

deiga commented 6 years ago

@hmshwt That sounds wonderful! :) 👍