refactor a keys test #160
refactor a keys test #160
Conversation
Signed-off-by: Adam Korczynski <[email protected]>
Signed-off-by: Adam Korczynski <[email protected]>
tuf_conformance/test_keys.py
Outdated
| roots keys and not check that the snapshot MD also has the | ||
| same keys. For example, the goal is to bring the repository |
There was a problem hiding this comment.
client might calculate the threshold from only the roots keys and not check that the snapshot MD also has the same keys
Careful naming here (and in the test name) would help in understanding the test: Snapshot metadata contains no keys so the sentence does not make sense.
So now reader is left wondering if the meaning is that snapshot role (in root metadata) should have the same keys as root role or that snapshot metadata should have signatures from snapshot keys defined in root metadata
There was a problem hiding this comment.
In terms of test name, wdyta test_root_meets_threshold_but_snapshot_does_not?
There was a problem hiding this comment.
I don't think it's great ("root meets threshold" normally means something completely different from what the name refers to). But let's merge and move on.
Signed-off-by: Adam Korczynski <[email protected]>
tuf_conformance/test_keys.py
Outdated
| repo.root.roles[Snapshot.type].threshold = 5 | ||
| repo.bump_root_by_one() # v5 | ||
| # Set the threshold to 4 and remove a signer from snapshot | ||
| # metadata so that root has 4 keys and snapshot has 3 |
There was a problem hiding this comment.
nit: this is still a really unclear comment:
root has 4 keys and snapshot has 3
snapshot role (in root metadata) contains 4 keys, snapshot metadata is signed by 3 keys
There was a problem hiding this comment.
Renamed and reworded in 7b3260a. LMKWYT.
Signed-off-by: Adam Korczynski <[email protected]>
initial_setup_for_key_threshold.initial_setup_for_key_threshold.test_root_has_keys_but_not_snapshot.test_root_has_keys_but_not_snapshotto demonstrate the intention of the test.