-
-
Notifications
You must be signed in to change notification settings - Fork 93
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
Add tests for proxy_study_artifact
#712
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #712 +/- ##
==========================================
+ Coverage 63.77% 64.62% +0.85%
==========================================
Files 35 35
Lines 2280 2284 +4
==========================================
+ Hits 1454 1476 +22
+ Misses 826 808 -18 ☔ View full report in Codecov by Sentry. |
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.
Thank you for your pull requests. I left some comments.
@@ -80,3 +88,49 @@ def test_list_trial_artifacts(init_storage_with_artifact_meta: MagicMock) -> Non | |||
{"artifact_id": "id1", "filename": "bar.txt"}, | |||
{"artifact_id": "id2", "filename": "baz.txt"}, | |||
] | |||
|
|||
|
|||
class TestProxyStudyArtifact(TestCase): |
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.
Could you use pytest instead of unittest's class-based style since we now use pytest as a test runner?
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 see. I have revised the code.
Co-authored-by: c-bata <c-bata@users.noreply.github.com>
Co-authored-by: c-bata <c-bata@users.noreply.github.com>
Co-authored-by: c-bata <c-bata@users.noreply.github.com>
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 to me 👍
Contributor License Agreement
This repository (
optuna-dashboard
) and Goptuna share common code.This pull request may therefore be ported to Goptuna.
Make sure that you understand the consequences concerning licenses and check the box below if you accept the term before creating this pull request.
Reference Issues/PRs
This PR adds tests for proxy_study_artifact.
What does this implement/fix? Explain your changes.
The added tests check the following: