Skip to content
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

Add test coverage for delegated hash bins target download #1910

Conversation

ivanayov
Copy link
Collaborator

This change adds tests coverage for path_hash_prefixes and
verifies that role names matching specific prefixed successfully
find and download the corresponding metadata files and do not
succeed to find existing, but non-matching files

Fixes #1639

Follow-up of #1808

  • The code follows the Code Style Guidelines
  • Tests have been added for the bug fix or new feature
  • [n/a] Docs have been added for the bug fix or new feature

@coveralls
Copy link

coveralls commented Mar 18, 2022

Pull Request Test Coverage Report for Build 2027942163

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 98.312%

Totals Coverage Status
Change from base Build 2022257279: 0.0%
Covered Lines: 1179
Relevant Lines: 1195

💛 - Coveralls

@ivanayov ivanayov force-pushed the delegated_hash_bins_tests_follow_up branch from 8247de1 to b32d345 Compare March 23, 2022 10:46
This change adds tests coverage for `path_hash_prefixes` and
verifies that role names matching specific prefixed successfully
find and download the corresponding metadata files and do not
succeed to find existing, but non-matching files

Signed-off-by: Ivana Atanasova <[email protected]>
@ivanayov ivanayov force-pushed the delegated_hash_bins_tests_follow_up branch from b32d345 to 02afa59 Compare March 23, 2022 10:58
@jku
Copy link
Member

jku commented Apr 20, 2022

I've not given this the review it needed, sorry... This vaguely looks like we could do better but I can't immediately see the better way: did you consider having the same test setup as in the existing downloads test test_targetfile_search() -- so there would be

  • staticly defined metadata structure: it could be the same delegations_tree already defined in the file, just added with a targets metadata that does hashed bin delegations
  • test cases would then only have to define a target path to search for, search result and the expected visited order

This would not make the current test much shorter but it might mean that adding a new test case would be maybe 2 lines instead of 40 lines of test data that it is now?

"delegate_1.ext",
),
],
visited_order=["f8-ff", "20-27"],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed you are not using visited_order anywhere in your test.

@lukpueh
Copy link
Member

lukpueh commented Aug 9, 2022

Is this superseded by #2031? Should we think about deprecating path_hash_prefixes now that TAP 15 has landed? (see see theupdateframework/taps#132 (comment))

@jku
Copy link
Member

jku commented Oct 10, 2022

Is this superseded by #2031?

I think so. I wouldn't object to a tight client test specifically for hashed bins but I don't think it's super important, and this PR is 80 lines of test code for very limited (new) test functionality.

@jku jku closed this Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RepositorySimulator: add support for delegated hash bins
5 participants