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
test(checker): add importchecker test #2042
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2042 +/- ##
===========================================
- Coverage 88.75% 60.44% -28.32%
===========================================
Files 200 181 -19
Lines 10933 10372 -561
===========================================
- Hits 9704 6269 -3435
- Misses 1229 4103 +2874
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Latency summaryCurrent PR yields:
Breakdown
Backed by latency-tracking. Further commits will update this comment. |
tmp_exec_driver = os.path.join(str(tmpdir), 'tmp_driver.yml') | ||
args.summary_exec = tmp_exec_file | ||
args.summary_driver = tmp_exec_driver | ||
ImportChecker(args) |
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.
what is being tested here?
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 want to test ImportChecker from checker.py and _print_dep_tree_rst from importer.py. ImportChecker will call _print_dep_tree_rst to write executors and drivers to tmp files.
What could be the best practice do you think to test it?
Like, assert the content in text files? Or is it possible to use mock?
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.
did not u say it is not used anywhere?
U could do check a little the text file or use a mock and see the arguments passed.
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 thought it's a private function, but actually, it's used outside of that file. And usually, we will do Jina check
without these summary_exec
, summary_driver
. I think that's the reason they are not tested before. But they offer the ability to output executors, drivers to files.
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.
so lets test the output of the functionality
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.
Sure. Will do!
No description provided.