rename log files and use pathlib.Path #30
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
taldcroft
reviewed
Oct 15, 2020
testr/packages.py
Outdated
| @@ -187,7 +187,10 @@ def run_tests(package, tests): | |||
| interpreter = test['interpreter'] | |||
|
|
|||
| logger.info('Running {} {} script'.format(interpreter, test['file'])) | |||
| logfile = Tee(test['file'] + '.log') | |||
| if test['file'].endswith('.sh'): | |||
There was a problem hiding this comment.
Can we just do logfile = Tee(Path(test['file']).with_suffix('.log')))? (Assuming Tee accepts a Path). The asymmetry of treating .sh doesn't spark joy.
There was a problem hiding this comment.
yes, I agree it is ugly. I will try your proposal.
There was a problem hiding this comment.
Tee passes the path directly to open and nothing else, so this line would work.
The log_file is used in another place, which uses other functions, so I have to decide how to change it. Pathlib is not used in this module. It uses os.path heavily.
taldcroft
approved these changes
Oct 16, 2020
Merged
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Rename log files from
*.sh.logto*_sh.logAlso replaced most uses of
os.pathbypathlib.Path.Testing
Passes unit tests on MacOS, linux, Windows (at least one required)(no unit tests)run_testrfrom within theska_testrdirectory and from../test_testrrelative toska_testrto check that: 1.- tests pass, 2.- logs are in the right place:run_testr --include Quaternion(test_spec path is None)run_testr --include dpa_check --root ../ska_testr(this uses regress_dir)run_testr --test-spec test_spec_Quaternion(using proper test_spec path)run_testr --test-spec test_spec_Quaternion --root ../ska_testr(logs should be inoutputswithin CWD)run_testr --test-spec test_spec_Quaternion --root ../ska_testr --outputs-dir other_output(logs should be inother_outputwithin CWD)run_testr --test-spec test_spec_Quaternion --root ../ska_testr --outputs-dirpwd_output(logs should be in the given absolute path within CWD)Fixes #