-
Notifications
You must be signed in to change notification settings - Fork 29
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
Add tests for the code #43
Comments
Some auto testing providers I know of: The project needs to be integrated with them. |
@kirtibajaj I guess as you are managing this product, so it should be done by you. |
do we still need it? |
Yes we still need tests.
…On Sat 26 Jan, 2019, 19:17 Shivank98 ***@***.*** wrote:
do we still need it?
i can work on it.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#43 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHVj0Y41dRpBAu9oeLXsghTSObkdKcB4ks5vHFxogaJpZM4aJF-X>
.
|
ok then, I am on it. do I need to keep something in mind for the approach ? as i don't have any experience with travis or circle, just little bit of unit tests. |
For now I think simply testing if the server runs is enough. Later on as need increases we can improve testing standards. |
that will catch all syntax errors and so on. |
while writing the tests i faced a error, i tried google too but didn't understand it. else it test for other two functions in
and this give me this error.
|
can we please stop having code in comments. Just submit a PR and we can discuss things there. |
sure sure, my bad |
@theSage21 i am back. is this issue still on ? |
This is still on. I haven't written any code I can remember for this |
Hey @theSage21, I think it would be better to use |
#68 is already doing that I think |
As found out in another discussion the project is now big enough that simply reading the code during review does not ensure that it will run.
For starters a simple test file is needed which can run the server and make sure it does not crash. Later on we can add more tests.
The text was updated successfully, but these errors were encountered: