-
Notifications
You must be signed in to change notification settings - Fork 0
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 Unittests for different components - [merged] #57
Comments
In GitLab by @as6520 on Sep 2, 2020, 15:08 unmarked as a Work In Progress |
In GitLab by @as6520 on Sep 2, 2020, 15:08 changed title from {-WIP: Add tes-}ts to {+Add Unittests for different componen+}ts |
In GitLab by @as6520 on Sep 2, 2020, 15:08 changed the description |
In GitLab by @as6520 on Sep 2, 2020, 15:53 changed the description |
1 similar comment
In GitLab by @as6520 on Sep 2, 2020, 15:53 changed the description |
In GitLab by @waxlamp on Sep 3, 2020, 11:12 Commented on tests/test_parinterface.py line 258 Don't you need an |
In GitLab by @waxlamp on Sep 3, 2020, 11:12 Looks good overall. |
In GitLab by @waxlamp on Sep 3, 2020, 11:12 approved this merge request |
In GitLab by @as6520 on Sep 3, 2020, 13:14 Commented on tests/test_parinterface.py line 258 Baseed on the function signature. I am not sure what the assertion would be upon |
In GitLab by @waxlamp on Sep 3, 2020, 13:53 Commented on tests/test_parinterface.py line 258 The problem is, as written, this test is not testing anything. You can leave it here but if session termination is not currently testable, we should maybe open an issue in tinker-engine to make it so. |
In GitLab by @as6520 on Sep 3, 2020, 14:03 Commented on tests/test_parinterface.py line 258 Sure, although wouldn't the issue be in sail-on-client and maybe on sail-on too in case the server needs to provide meaningful response? |
In GitLab by @waxlamp on Sep 3, 2020, 14:07 Commented on tests/test_parinterface.py line 258 You're right, wherever the Just to be clear, I approved this particular MR already so you can merge it when you're ready. |
In GitLab by @as6520 on Sep 3, 2020, 14:08 Commented on tests/test_parinterface.py line 258 Sounds good #23 would address the issue |
In GitLab by @as6520 on Sep 3, 2020, 14:08 Commented on tests/test_parinterface.py line 258 Waiting on Adam's review too |
In GitLab by @awk11 on Sep 3, 2020, 15:34 Commented on tests/test_parinterface.py line 258 Sorry for the delay, today's been busy. As for this question though, terminate session simply returns either the string "OK" or passes along any error on the api end. So you can either have the client side of it pass that along if you want to be able to assert against something, or can just leave it as is and the test is to ensure that no errors are thrown. This is the same in post_results |
In GitLab by @awk11 on Sep 3, 2020, 15:38 Everything looks good to me |
In GitLab by @awk11 on Sep 3, 2020, 15:38 resolved all threads |
In GitLab by @as6520 on Sep 3, 2020, 15:40 Commented on tests/test_parinterface.py line 258 Thanks for the info, I can use "OK" and update the tests in two methods along with two methods in #23 |
In GitLab by @as6520 on Sep 3, 2020, 15:41 merged |
In GitLab by @as6520 on Sep 2, 2020, 13:48
Merges add-tests -> master
This MR adds unittests for different components of the package. The tests are run using pytests in CI.
Closes #15, #21
The text was updated successfully, but these errors were encountered: