-
Notifications
You must be signed in to change notification settings - Fork 15
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
Unit testing #114
Comments
Good time to surface this concern @maxceem. @sachin-maheshwari is already in process of kind of rewriting this application as generic as possible and make it independent of Connect or any other app. So, it might be good time for introducing the unit tests with the new code he is writing. |
Great to know about code rewriting. Maybe it could worth introducing unit tests before the code is rewritten if we can reuse these unit tests in the new code too. This way we could verify that new code keeps the functionality we have in the old code. But it would depend on how the new code is different from the old code if we can reuse unit tests or no. Basically, our unit tests may work more like end-to-end tests:
So if we check end-to-end cases like described above, there is a big chance they would work in the new code also. |
Yes, ideally integration tests should work just fine even after rewrite. However, unit tests would be significantly different I guess here because code rewrite would require deleting few old files, introducing new files and essentially it is introducing a framework kind of classes/files which would allow introducing more and more applications to consume notifications easily. Also, for integration tests, I would suggest (for this particular repo only) to have some kind config file (json,csv or something more appropriate) to have mapping of what (db notification, email notification) is expected for a particular bus event. It can be complicated though considering that notifications are per user, so we might need to be aware of test setup data when creating/editing this config file. |
This project doesn't have unit testing setup at the moment.
It would be good to have unit tests for this project because:
The text was updated successfully, but these errors were encountered: