Modernized and update test.py #7
Conversation
The changes made to the code are: - Added a check to ensure that the `needledir` directory exists before attempting to read its contents. This is done using the `os.path.exists()` function. Removed the use of the `global` keyword for the `returncode` variable and instead passed it as an argument to the `error()` function. Simplified the code that checks for the existence of JSON and PNG files for each needle. Instead of using multiple `if` statements, a single `if` statement is used with the `continue` keyword to skip to the next iteration of the loop if the JSON file does not exist. Replaced the regular expression used to extract the timestamp from the needle name with the `str.split()` method. This method splits a string at a specified separator and returns a list of the resulting substrings.
|
Hey @asdil12 please review it |
| error("Needle '{}' includes a workaround tag but has no reason in json file!".format(needle)) | ||
| break | ||
| properties = n.get('properties', []) | ||
| if properties is not None: |
There was a problem hiding this comment.
This should never evaluate to false, as the .get() method returns a default of [] here.
| workaround_found = True | ||
| break | ||
| if not workaround_found: | ||
| error(f"Needle '{needle}' includes a workaround tag but has no bug-ID in filename or reason in json file!") |
There was a problem hiding this comment.
I would prefer the old approach of individual messages.
No reason to summarize two different issues.
Let's let the user know exactly, what they did wrong.
| @@ -1,12 +1,10 @@ | |||
| #!/usr/bin/python3 | |||
There was a problem hiding this comment.
Please leave the shebang - the script won't work otherwise.
|
|
||
| # Check if name contains timestamp | ||
| timestamp = re.sub(r"_.*$", '', re.sub(r"[_-][0-9]{1,2}$", '', needle).split("-")[-1]) |
There was a problem hiding this comment.
IIRC this regex was used to replace suffixes, that would remain in your simpler approach.
Eg if a needle was called foo-20201231-01 or foo-20201231_bar, the simpler approach would fail.
Do you have a reason to expect that we won't have needles anymore with such names?
| import os | ||
| import sys | ||
| import re | ||
| import json | ||
|
|
||
| scriptdir = os.path.dirname(os.path.realpath(__file__)) | ||
| needledir = os.path.join(scriptdir, "..") | ||
| needledir = os.path.abspath(os.path.join(scriptdir, "..")) |
The changes made to the code are:
needledirdirectory exists before attempting to read its contents. This is done using theos.path.exists()function.globalkeyword for thereturncodevariable and instead passed it as an argument to theerror()function.ifstatements, a singleifstatement is used with thecontinuekeyword to skip to the next iteration of the loop if the JSON file does not exist.str.split()method. This method splits a string at a specified separator and returns a list of the resulting substrings.