- Tests are quite long for what they do
- They seemingly test for the same things multiple times
- while also doing things that are not relevant to the documented test case -- maybe testing for some other possible failure?
- initial_setup_for_key_threshold is not just shared initialization, it contains actual tests
I'd much rather see multiple minimal tests for specific purpose.
test_root_has_keys_but_not_snapshot() is an example:
-
starts by testing threshold failure twice (once in the initialization function), once in the test itself even though there is a simpler test_snapshot_threshold() that tests for threshold failure already
-
the actual test as documented seems to be this:
repo.root.roles[Snapshot.type].keyids.append(signer.public_key.keyid)
...
# Updating should fail. Root should bump, but not snapshot
assert client.refresh(init_data) == 1
-
there's issues here:
- a new root is never published so the client can never see the modified root
- adding just a keyid is not correct -- it's fine to test for in one test but here, just like in 99% of cases, you want to add the actual key with root.add_key()
- there is no way snapshot could bump to anything even if the client was buggy somehow: snapshot remains at the initialized version 3 through the test so the comment makes no sense
It seems like the test as documented could be ~ten lines of code: I suggest using --repository-dump-dir to see what kind of repo you are actually generating.
I'd much rather see multiple minimal tests for specific purpose.
test_root_has_keys_but_not_snapshot() is an example:
starts by testing threshold failure twice (once in the initialization function), once in the test itself even though there is a simpler
test_snapshot_threshold()that tests for threshold failure alreadythe actual test as documented seems to be this:
there's issues here:
It seems like the test as documented could be ~ten lines of code: I suggest using --repository-dump-dir to see what kind of repo you are actually generating.