-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: JumpStart list models flaky tests #4525
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4525 +/- ##
=======================================
Coverage 87.38% 87.38%
=======================================
Files 389 389
Lines 36776 36776
=======================================
+ Hits 32135 32136 +1
+ Misses 4641 4640 -1 ☔ View full report in Codecov by Sentry. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
@@ -393,7 +393,9 @@ def _generate_jumpstart_model_versions( # pylint: disable=redefined-builtin | |||
if isinstance(filter, str): | |||
filter = Identity(filter) | |||
|
|||
manifest_keys = set(models_manifest_list[0].__slots__) | |||
manifest_keys = set( |
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.
that's a bit concerning that this does not cause any unit tests to fail
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.
It's because these keys are exactly the same. I was thinking to add a hypothetical new key in a test, but then I feel it should be done when we actually change the slots in JumpStartModelHeader
. Let me know if you feel strongly to add it now
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 you're say it happens to work now, cause they're the same. i guess that's fine. what about the spec keys though?
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
* fix list models flaky tests * fix
* fix list models flaky tests * fix
Issue #, if available:
Description of changes:
This change fixes the list models flaky unittests by making sure the mocks are set up correctly.
Testing done:
Merge Checklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.General
Tests
unique_name_from_base
to create resource names in integ tests (if appropriate)By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.