-
Notifications
You must be signed in to change notification settings - Fork 4
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
Unittests for meta- and resultnode #20
Conversation
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.
LGTM.
I added a few comments. But the overall changes are very good.
A few general things:
- I think a few sporadic docstrings in the
conftest.py
would help the overall readability - I would also add return type hints for all fixtures, This will simplify the refactorings later on.
- Often there are more empty lines than needed. Can you check that there are no unnecessary empty lines? (two lines between classes and static functions, a single line between functions within classes.)
- Can you add a description to the MR
- Can you make a better commit message? (https://cbea.ms/git-commit/)
3477831
to
a469177
Compare
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.
🥇
a469177
to
0329063
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #20 +/- ##
============================================
- Coverage 100.00% 85.44% -14.56%
============================================
Files 16 16
Lines 1292 1292
Branches 218 218
============================================
- Hits 1292 1104 -188
- Misses 0 179 +179
- Partials 0 9 +9
☔ View full report in Codecov by Sentry. |
use mocking and fixtures for cleaner unittests. Old tests are being replaced.
0329063
to
f2ad5b7
Compare
Adding unittests for MetaNode and ResultNode.