-
Notifications
You must be signed in to change notification settings - Fork 276
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
More trusted set tests #1522
More trusted set tests #1522
Conversation
* test for succesful root update * fix test for snapshot version rollback in timestamp update Signed-off-by: Jussi Kukkonen <[email protected]>
Test failing length/hash test for snapshot Signed-off-by: Jussi Kukkonen <[email protected]>
We're unlikely to use the iterator but it's required to implement Mapping. Signed-off-by: Jussi Kukkonen <[email protected]>
Signed-off-by: Jussi Kukkonen <[email protected]>
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.
The changes are simple and make sense.
LGTM!
tests/test_trusted_metadata_set.py
Outdated
def test_update_timestamp_snapshot_ver_below_current(self): | ||
def bump_snapshot_version(timestamp: Timestamp) -> None: | ||
timestamp.meta["snapshot.json"].version = 2 | ||
|
||
# set current known snapshot.json version to 2 | ||
timestamp = self.modify_metadata("timestamp", bump_snapshot_version) | ||
self._root_updated_and_update_timestamp(timestamp) |
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 made a mistake when writing this test and test_update_timestamp_new_timestamp_ver_below_trusted_ver test.
It must have been a mistake while rebasing.
Great catch!
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.
Nice set of test enhancements, LGTM
This gets us to 99% coverage (the only missing thing is successful rollback test -- IOW successfully updating a new snapshot when a snapshot is loaded already -- this requires #1498 I think)