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

RedisAdapter always opens a connection and does not offer a way to close it #137

Open
fgblomqvist opened this issue Oct 27, 2022 · 2 comments

Comments

@fgblomqvist
Copy link
Contributor

Fiddling around with the tests I kept wondering why mocha kept running forever even though all the cases finished. Any test file that imports from the testConfig keep running forever, even if it just just a expect(true).to.eql(true).

I discovered that testConfig imports the Redis adapter from redis-adapter.js and that that file in turn has a side effect of opening a redis connection, no matter what. Furthermore, there is no way to close that connection since the client sits unexported in the global scope.

There are several issues at play here:

  • The client should not connect to anything on import, that is bad practice.
    • Instead, make the connection inside the RedisAdapter constructor (why otherwise have a constructor?) and store it on this (rather than globally). If you want a single connection, you can initialize a single instance of this adapter somewhere and pass it around (or at least do a singleton pattern).
  • It is not possible to close the connection.
    • Add a method to the adapter that calls the this.client.disconnect() method. Once that is in place, any user of it (such as tests and application code) can now properly close the connection at the end.
@simonv3
Copy link
Collaborator

simonv3 commented Oct 27, 2022

I think cleaning up the Redis adapter is a good idea, but I'm not experiencing the issues with mocha keeping on running forever except that watch mode is on by default. How are you running the tests?

@fgblomqvist
Copy link
Contributor Author

fgblomqvist commented Oct 31, 2022

Even with watch mode turned off it happens. I do yarn docker:compose:test:up and then in a separate window yarn test:all. After saying how many passed and skipped it just hangs. Sounds quite odd that our setups are running so differently 🤔

If I run a single test that does not do any DDB stuff, such as the Template.test.js, and after I've fixed the redis client, that test finishes without issues and mocha exits. But yeah, lots of other oddities. If nothing else, it's fascinating debugging this lol.

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

No branches or pull requests

2 participants