Remove test_updater_with_simulator.py #1741
Conversation
|
|
Pull Request Test Coverage Report for Build 1607257357Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Remove TestUpdater.test_refresh from test_updater_with_simulator. Testing refresh() is now extensively covered in the newly added test_updater_top_level_update.py. Signed-off-by: Teodora Sechkova <tsechkova@vmware.com>
Move test_fishy_rolenames to test_updater_delegation_graphs.py and update the test setup. Signed-off-by: Teodora Sechkova <tsechkova@vmware.com>
Move test_not_loading_targets_twice to test_updater_top_level_update.py. Signed-off-by: Teodora Sechkova <tsechkova@vmware.com>
Key rotations and metadata update are now extesively tested in: - test_updater_key_rotations.py - test_updater_top_level_update.py Signed-off-by: Teodora Sechkova <tsechkova@vmware.com>
Move the remaining test_snapshot_rollback_with_local_snapshot_hash_mismatch to test_updater_top_level_update.py and remove the file. Signed-off-by: Teodora Sechkova <tsechkova@vmware.com>
sechkova
force-pushed
the
remove-updater-with-sim
branch
from
4263164 to
672df74
Compare
sechkova
changed the title
| @patch.object(builtins, "open", wraps=builtins.open) | ||
| def test_not_loading_targets_twice(self, wrapped_open: MagicMock) -> None: | ||
| # Do not load targets roles more than once when traversing | ||
| # the delegations tree | ||
|
|
||
| # Add new delegated targets, update the snapshot | ||
| spec_version = ".".join(SPECIFICATION_VERSION) | ||
| targets = Targets(1, spec_version, self.sim.safe_expiry, {}, None) | ||
| role = DelegatedRole("role1", [], 1, False, ["*"], None) | ||
| self.sim.add_delegation("targets", role, targets) | ||
| self.sim.update_snapshot() | ||
|
|
||
| # Run refresh, top-level roles are loaded | ||
| updater = self._run_refresh() | ||
| # Clean up calls to open during refresh() | ||
| wrapped_open.reset_mock() | ||
|
|
||
| # First time looking for "somepath", only 'role1' must be loaded | ||
| updater.get_targetinfo("somepath") | ||
| wrapped_open.assert_called_once_with( | ||
| os.path.join(self.metadata_dir, "role1.json"), "rb" | ||
| ) | ||
| wrapped_open.reset_mock() | ||
| # Second call to get_targetinfo, all metadata is already loaded | ||
| updater.get_targetinfo("somepath") | ||
| wrapped_open.assert_not_called() |
There was a problem hiding this comment.
I am wondering if this is the correct file for this test or it should be test_updater_delegation_graphs?
It seems we are actually using and testing delegation roles here.
There was a problem hiding this comment.
it does define a delegation but the point is just to test file opens (so not specifically delegations or top level updates)... If this is the only issue with the PR, I think I'll merge as is and we can improve later
| for fname in roles_to_filenames.values(): | ||
| self.assertTrue(fname in local_metadata) | ||
|
|
||
| def test_keys_and_signatures(self) -> None: |
There was a problem hiding this comment.
There are some tests covering keys and signatures (like test_new_timestamp_fast_foward_recovery, test_root_rotation, etc) , but I'm not sure they follow the exact specific case covered here. I'm fine with deleting it though, if others find it unnecessary.
There was a problem hiding this comment.
I think this is fine: this specific test was just me adding an example of how to modify sigs and keys when using the simulator. I think the simulator documentation and the better tests that now exist cover all that better by now.
WIP since it is built on top of two other PRsDescription of the changes being introduced by the pull request:
test_updater_with_simulator.pywas initially added to to demonstrate the usage ofRepositorySimulatorwhen testingUpdater. Now Updater is tested extensively using simulator in several other test files.This PR removes the duplicated tests and moves the useful ones to the relevant test file.
Please verify and check that the pull request fulfills the following
requirements: