Skip to content
This repository has been archived by the owner on Nov 27, 2018. It is now read-only.

Auto-close dynalite when tape is finished #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

agius
Copy link

@agius agius commented Mar 15, 2018

Managing dynamodb.start() and dynamodb.close() across multiple files is difficult.

  • If I have dynamodb.start() at the beginning of each file, and dynamodb.close() at the end of each file, I'll get ECONNREFUSED on the first test in the second file.
  • Putting dynamodb.close() inside any test function causes an infinite hang
  • Using tape.onFinish() creates an infinite loop, since close() creates a new test
  • Trying process.on('beforeExit') or process.on('exit') will also hang infinitely, since tape hijacks the regular process exit to turn callback functions into serial tests
  • There is no way to manually shut down the dynalite server, since it is not exposed from the module and there is no singleton
  • My current solution is to make a new file called zzzzzzzzzz-close-dynamodb.test.js and hope that is lexicographically last across machines.

With this PR I could delete that file since the server will shut itself down once all tests are complete. I could also extract the dynalite instance and do any necessary jiggery-pokery.

Copy link
Owner

@rclark rclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has always been a nightmare. Have you been able to deduce the cause of this hang?

Putting dynamodb.close() inside any test function causes an infinite hang

That's not something I've observed before.

@@ -71,6 +72,7 @@ function ddbtest(test, projectName, tableDef, region) {
dynalite.listen(4567, function(err) {
if (err) throw err;
listening = true;
test.onFinish(function(){ dynalite.close(); });
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this cause any problems if dynalite is already closed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, not in my tests.

@agius
Copy link
Author

agius commented Mar 15, 2018

Admittedly, my method of "testing" is not terribly rigorous. I was splitting tests into multiple files for this PR (routing.test.js was a misnomer, big, and growing fast).

I did a bunch of raw edits to node_modules/dynamodb-test/index.js to try and figure out what was going on. Adding this fixed my issue, and allowed me to remove dynamodb.start() and .close() from everywhere in the test files. Left in my zzzz-* file with .close() to test out the double-close-dynalite case.

Not really sure how to build a robust test around "does this close out correctly when mocked" and still have the manual close tests that are already here

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

Successfully merging this pull request may close these issues.

2 participants