-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
define pytest logs directory #491
define pytest logs directory #491
Conversation
Category: feature JIRA issue: MIC-5503 Add with checks. Testing Added multiple assert statements that would fail using with check and verified that they all appeared in the output logs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! I do think you should add descriptive IDs. I'm also concerned about a couple other small things, so be sure to address those before actually merging.
@@ -16,13 +16,31 @@ | |||
(["--dataset", "wic", "--year", "2015"]), | |||
# (["--dataset", "wic", "--population", "USA", "--state", "RI", "--year", "2015"]), | |||
], | |||
ids=["1", "2", "3", "4"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd talked at some point about automating these IDs to that (1) they are more descriptive and (2) so that they auto-adjust when you add/remove parameterizations. Maybe: ids=lambda x: "_".join([item for item in x if not item.startswith("--")]),
"--release", | ||
"test_release.py", | ||
"--check-max-tb=1000", | ||
"--population", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you hard-coding population instead of parametrizing it?
assert result.returncode == 0 | ||
|
||
# log using job id | ||
job_id = request.node.callspec.id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is even more of a reason to descriptively handle job IDs. pytest_wic_2015.o
is a lot more informative than pytest_4.o
.
@@ -16,13 +16,31 @@ | |||
(["--dataset", "wic", "--year", "2015"]), | |||
# (["--dataset", "wic", "--population", "USA", "--state", "RI", "--year", "2015"]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these all be uncommented?
define pytest logs directory
Description
Add output-dir option when running pytest.
Create profiling directory using this output directory.
Update test_runner to write logs.
Testing
Ran test_runner.py and checked that the logs directory and profiling directory were created and populated as expected.