-
Notifications
You must be signed in to change notification settings - Fork 178
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
fix(shared-data): Fix the pipette schemas, tests #13949
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## edge #13949 +/- ##
==========================================
- Coverage 70.79% 70.76% -0.04%
==========================================
Files 2505 2478 -27
Lines 70620 69876 -744
Branches 8663 8501 -162
==========================================
- Hits 49997 49447 -550
+ Misses 18501 18332 -169
+ Partials 2122 2097 -25
Flags with carried forward coverage won't be shown. Click here to find out more. |
The tests that were supposed to ajv the pipette definitions weren't set up properly, and the tests that were supposed to test that the tests were set up properly weren't set up properly, so we weren't testing the schemas so the schemas were wrong. Fix the schema-test setup, the test-setup-schema-test setup, and the schema.
83635d1
to
f2ca4d0
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.
Thank you!
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
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.
oopsie and lgtm
@@ -18,6 +18,7 @@ def test_check_all_models_are_valid() -> None: | |||
"ninety_six_channel": "96", | |||
"eight_channel": "multi", | |||
} | |||
assert os.listdir(paths_to_validate), "You have a path wrong" |
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.
Is this necessary?
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.
IMO yes, we do it in the js tests too
The tests that were supposed to ajv the pipette definitions weren't set up properly, and the tests that were supposed to test that the tests were set up properly weren't set up properly, so we weren't testing the schemas so the schemas were wrong. Fix the schema-test setup, the test-setup-schema-test setup, and the schema.
The tests that were supposed to ajv the pipette definitions weren't set up properly, and the tests that were supposed to test that the tests were set up properly weren't set up properly, so we weren't testing the schemas so the schemas were wrong. Fix the schema-test setup, the test-setup-schema-test setup, and the schema.