feat: upgrade test packages to jest #19
Conversation
The return value of `findOneAndDelete` can be null, causing (modern) typescript to raise `TS2531: Object is possibly 'null'`
Between 5 -> 6, the return value of `findOneAndDelete` changed. It now returns the document or null directly, see 'https://github.com/mongodb/node-mongodb-native/blob/HEAD/etc/notes/CHANGES_6.0.0.md#findoneandx-family-of-methods-will-now-return-only-the-found-document-or-null-by-default-includeresultmetadata-is-false-by-default' By pinning the version we'll mitigate potential breaking changes in future.
The packages nyc and expect.js didn't have any active commits in many years. jest seems like a sensible choice to switch to. In future, the tests might be better refactored to remove the done callback pattern and use proper promises. Another thing I couldn't easily fix is the beforeEach delay of 200ms, this makes tests unnecessarily slow.
|
I'd rather not change something that is currently working. Did you have any issue with the current setup?
True! The delay is used to ensure that the MongoDB stream is properly initialized, not sure how we could improve this... |
|
I appreciate your response. While the current setup works, I suggested these changes with the intention of improving the long-term sustainability of the project. I believe it's important to consider the long-term viability of using packages that have not seen active development in many years. This can lead to potential security vulnerabilities and compatibility issues down the road. Personally, I couldn't even run the test suite locally. Might be some incompatibility between my node version but didn't really bother to look into it. It's worth emphasizing that these changes only affect packages related to testing, so the likelihood of breaking any critical functionality is minimal. Additionally, the PR is part of a broader effort to modernize the codebase. One of the planned future steps is to refactor the code to use promises instead of "fire and forget" (#15). I understand your concerns about introducing changes, especially when they might affect existing functionality. If you have any specific reservations or questions about the proposed changes, I'd be more than happy to address them and work together to find the best way to move forward. |
|
In that case, let's not waste our mutual time and focus on the things that matters. |
The packages nyc and expect.js didn't have any active commits in many years. jest seems like a sensible choice to switch to.
In future, the tests might be better refactored to remove the done callback pattern and use proper promises. Another thing I couldn't easily fix is the beforeEach delay of 200ms, this makes tests unnecessarily slow. That might be something for #15
Note: this branch continues on the work of #18. Not sure how you want to review/release this, but I splitted the PRs anyways. I can imagine you might want to bump the version of #18 as its sort-of backwards incompatible by requiring a mongodb version.