feat: handle logging for multiple yamls#263
Conversation
Codecov Report
@@ Coverage Diff @@
## master #263 +/- ##
=======================================
Coverage 96.91% 96.91%
=======================================
Files 77 77
Lines 6191 6194 +3
=======================================
+ Hits 6000 6003 +3
Misses 191 191
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
|
||
| :return: file name without extension | ||
| """ | ||
| full_name = os.path.basename(path) |
There was a problem hiding this comment.
With pathlib this would become:
return Path(path).resolve().name
There was a problem hiding this comment.
Actually Path(config_file).stem does exactly what I want and this function is useless 🤦♂️
There was a problem hiding this comment.
this function will be then removed ?
| @click.option( | ||
| "-l", | ||
| "--log-path", | ||
| "--log-paths", |
There was a problem hiding this comment.
I wouldn't introduce a breaking change on the name just because we are now supporting multiple ones, only the option's help should be extended IMO
| ) | ||
| log_index += 1 | ||
| else: | ||
| raise ValueError( |
There was a problem hiding this comment.
Consider using click.UsageError or any click CLI-specific exception instead
| if log_paths: | ||
| yaml_name = get_file_name_without_extension(config_file) | ||
| # Put all logs in one file | ||
| if len(log_paths) == 1: |
There was a problem hiding this comment.
Maybe run these check before looping and use something like:
for idx, config_file in enumerate(test_configuration_file):
logger = initialize_logging(
log_paths[idx], log_level, verbose, report_type, yaml_name
)
There was a problem hiding this comment.
Added the enumerate, for the check before the looping not sure it would make the code cleaner
| if log_path.is_dir(): | ||
| fname = time.strftime("%Y-%m-%d_%H-%M-test.log") | ||
| if log_path.suffix == "": | ||
| if not log_path.is_dir(): |
There was a problem hiding this comment.
Maybe more
if log_path.is_dir():
log_path.mkdir(exist_ok=True, parent=True)
?
There was a problem hiding this comment.
Well unless I miss something if I do this is_dir will return False if the folder does not exist and it will not be created
There was a problem hiding this comment.
Absolutely right!
You could still flatten this a bit by doing:
if log_path.suffix == "":
log_path.mkdir(parents=True, exist_ok=True)
which would already take care of the additional is_dir condition
3 participants
No description provided.