Unit tests have interdependencies about superlogin HOT 7 CLOSED

@tohagan Thank you very much for starting the discussion. I understand that many consider dependencies as an anti-pattern for unit testing. In SuperLogin most of the functionality is dependent on other functionality. For example logging in a user is dependent on creating a user. In order to test log out functionality, it is necessary to log in first.

Rather than wipe the database, create a new user, and initiate a session inside each test I chose to simply make them dependent on each other. From my point of view, this makes the code DRYer and easier to maintain. The tests run faster. Running each test sequentially (using previous.then(...)) ensures there are no database write conflicts.

My debugging workflow is to always handle the first failure until everything passes. If you have any concrete suggestions on how to improve I am open to a pull request.

from superlogin.

I'm sorry this sounded rather negative of me ... I'm really liking superlogin and your unit tests are very good.

What I eventually did was just convert the leading and trailing it() tests into before() and after() methods that setup and tear down the database. For performance I'm not using beforeEach()/afterEach(). This keeps your DRY pattern and ensures that we can run tests independently. You can still use previous.then() as before. When a unit test is failing I just make it() into it.only(function() ... )

Sometimes I make the db tear down be the 1st step in before() ... as this leaves the updated database at the end of the tests so I can inspect it when a test fails.

I've also merged changes from some forked projects. One in particular has fixed a bunch of issues I had getting your test suite working (my localhost CouchDb is not Admin Party and this broke stuff).

from superlogin.

Here's the patch that fixed the unit tests (not my code)

View:
ybian@24959f5

Patch:
https://github.com/ybian/superlogin/commit/24959f52db0774ce236775cb8b245889189832e6.patch

from superlogin.

Another observation is that you often add this at the end of a test ...

 .then(function() { done(); }).catch(function(err) { done(err); }) 

But in Mocha you don't need to since you can just return a promise and mocha will do this for you.
You don't even need to pass the done arg. Same for Jasmine if you use a plugin.

See "Working with Promises" under https://mochajs.org/

from superlogin.

@tohagan thank you very much for all the feedback! I am actually relatively new to the Javascript and testing scene and this is my first major project.

Within the next couple weeks I will make some time for the next release of SuperLogin and will take all this into consideration to clean up the test code.

from superlogin.

Sent you a PR that resolves this issue.

from superlogin.

Thank you for all you help @tohagan !
I am going to close this for now. If you have further feedback on how to eliminate test interdependency feel free to re-open. But since the setup and teardown takes time my preference is to leave it as is.

from superlogin.