Refactor tests to remove the embedded git - #32
ianthomas23 merged 12 commits into
Conversation
Sorry, something went wrong.
ianthomas23
left a comment
There was a problem hiding this comment.
Just a couple of niggles, otherwise it looks like a big improvement.
Sorry, something went wrong.
|
I've thought of one possible improvement, but this is already a big enough PR so it could be done separately. Rather than use the hardcoded path test/data for the tests, we could instead use the pytest fixture tmp_path like we do at Line 7 in 94b9870 Then each test will definitely be isolated from the other tests (each will have its own working_dir), and we won't have to cleanup after each test (e.g. removing the cloned xtl repo) as the temporary directories will automatically be cleaned for us. Also I will create an issue for checking the returncode after each subprocess.run call in the tests. |
Sorry, something went wrong.
| p = subprocess.run(cmd, capture_output=True, cwd="test/data/status_data", text=True) | ||
| assert(p.stdout == '* main\n') | ||
| p = subprocess.run(cmd, capture_output=True, cwd=working_dir, text=True) | ||
| print() |
There was a problem hiding this comment.
Ooh, empty print statement. I assume it isn't needed?
Sorry, something went wrong.
There was a problem hiding this comment.
I always forget a print left in my code somewhere...
Sorry, something went wrong.
There was a problem hiding this comment.
At least there were no swear words in it!
Sorry, something went wrong.
|
I'll change to tmp_path in another PR as suggested. |
Sorry, something went wrong.
ianthomas23
left a comment
There was a problem hiding this comment.
Thanks @SandrineP
Sorry, something went wrong.
Refactor tests to remove the embedded git