Restructuring and adding tests#8
Conversation
oraNod
force-pushed
the
pytest-unit-read-file
branch
from
be8c210 to
d1ff364
Compare
|
@FrostyX Hey I know this has been sitting around for a super long time but I'd like to revive it. Hopefully the restructure and direction makes sense. I'd like to add some more tests but I'll hold off for now and do that in follow on PRs. There's already a bunch of changes in here. And figuring out the tests has been challenging... |
|
Thank you for the PR @oraNod, I will try to review once I can |
No panic. I might make some changes once again. I'm unsure about using MagicMock now. It might be total overkill and unnecessary for all that abstraction. Maybe it's sort of a roundabout way of doing things. Cheers, man. |
|
|
||
| # Test repository names | ||
| project-for-testing-tito/ | ||
| wireplumber/ |
There was a problem hiding this comment.
It would be better if we had a git-ignored dst/tmp/output/something directory and clone the repositories there. That way we won't have to list all of them explicitly.
| import pytest | ||
|
|
||
| TESTS_DIR = Path(__file__).parent | ||
| JSON_DIR = TESTS_DIR / "json" |
There was a problem hiding this comment.
I had absolutely no idea we can join paths like this. That's kinda cool.
| file_path = JSON_DIR / "github-payload.json" | ||
| with open(file_path, encoding="utf-8") as file: | ||
| return json.load(file) | ||
| except FileNotFoundError: |
There was a problem hiding this comment.
Under what circumstances can this happen? The file_path isn't affected by any arguments and is always the same.
There was a problem hiding this comment.
I'll remove this to make it simpler. I was probably just being overzealous.
| data = load_json_data(mock_args_file) | ||
|
|
||
| assert data == gitlab_payload | ||
| mock_open.assert_called_once_with("payload.json", "r", encoding="utf-8") |
There was a problem hiding this comment.
It's IMHO not worth having both test_load_from_file_github and test_load_from_file_gitlab. The test only makes sure that load_json_data chooses the correct place to load the data from (the file in this case). There is no way that one of the tests passes and the other one doesn't.
The same for test_load_from_url_github and test_load_from_url_gitlab below.
| data = load_json_data(mock_args_url) | ||
|
|
||
| assert data == gitlab_payload | ||
| mock_get.assert_called_once_with("http://example.com/payload.json") |
There was a problem hiding this comment.
I think the most valuable tests will be for extract_repository_url, extract_project_name, and parse_commits. And at the same time, those will be the easiest to write :-)
There was a problem hiding this comment.
Yeah, now that the basic pytest setup is in there I hope those will come easily. I'll do that as a follow on PR though.
|
Thanks for the review @FrostyX I've pushed a couple of commits here to address things. I feel like there were a couple more things to simplify but I've forgotten pretty much everything after summit. If this looks good, I'd like to merge and handle all other refactoring and new tests in follow on PRs. Cheers! |
2 participants
This PR adds some initial tests.