Unit tests Improvement #541
Conversation
|
Btw, I do notice that the one of the latest changes in the current master branch has failed tests on it and I have fixed it on my branch via #5. Was curious if the test pipeline is not working at the moment? |
There was a problem hiding this comment.
Hey, thanks a lot for this PR. I understand that it's a lot of work to get started on writing a comprehensive unit test suite, and I appreciate the effort. I added many comments, and I really hope that we can resolve them all because I would very much like to land this PR.
|
Hi @regisb , thanks for the feedback, I will look at the mentioned comments and adapt them by this weekend. |
|
Hi @regisb , please find all of the changes mentioned amended as in this PR on my own fork. |
|
Hi and happy new year @regisb , just updated the PR with the latest master and resolved conflicts. I also reverted those remaining unresolved comments. |
|
I moved my comments to a separate PR: takato1314#9 |
|
Hi @regisb. I've done approving and merging PR: takato1314#9 |
|
Awesome! Can you please rebase your changes on top of master and squash all your commits? ( |
|
Please dont approve first. There are some mistakes while rebasing the commits |
|
Ok let me know if you are facing issues with the rebase. If necessary I can rebase myself. |
Update gitignore file Use black to format code and fix unit test for test_pathjoin to use os.path.join() instead of hardcoding the path. Changes: - Remove #!shebang from the codebase because it will force the integrated terminal to you the global installed python instead of the one defined in venv. - Add launch.json Add coveragepy for measuring code coverage for tutor. Add click command tests Add unit tests for utils.py while fixing some issues encountered when running the tests on win32 platform Add unit tests for test_config and test_plugins Add unit tests for test_context and test_images Add tests to test_config Created TestContext used for unit testing Add more tests to test_config Add tests to test_jobs Remove unnecessary cleanup process from TestContext in test.py because we are using static CONTEXT now for the entire test suite to improve performance. Remove notebooks from unrelated branch Fix script errors on jobs.py Revert jobs.py to support code templating and write tests for jobs Improve unit tests for utils Fix unit test issue from recent changes on master. Fix issues on unit tests from latest master #8f90cc1 Fix all changes required for [PR#541](overhangio#541 (review)) Remove .vscode IDE settings from PR. Fix failing unit tests from the latest master merge Remove load_dev_requirements() Revert to use POpen.Wait() instead of Communicate(). Add PR comments as code Instead of adding many comments on top of the existing PR, I thought I could make the changes myself and push them on top of the existing PR. If you agree with these changes, just merge them in your branch and they will be reflected in your PR.
|
Hi @regisb. Sry for some delay. Finally figured out the rebase and able to run the tests successfully on my local machine. I also did a few quick start tests, seems to be working. |
|
Just a quick question, I saw this dependency error while install on windows with the command p/s: Sorry forgot to attach image |
|
Ok I missed the failing error while running |
|
@takato1314 I think that you have multiple errors in your rebase. I wil rebase your changes locally and will push the commit myself if that's ok with you. |
|
Yes please do so I shall close this PR then. |
Add unit tests to the tutor project from 60 tests to 121 tests with a total code coverage of 63.57%. The detailed changes are described in the following pull requests in my committing repo: