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

Design test strategy #1579

Closed
joshuagl opened this issue Sep 14, 2021 · 10 comments
Closed

Design test strategy #1579

joshuagl opened this issue Sep 14, 2021 · 10 comments
Assignees
Labels
backlog Issues to address with priority for current development goals testing
Milestone

Comments

@joshuagl
Copy link
Member

joshuagl commented Sep 14, 2021

Description of issue or feature request:

The following list captures some areas of interest for testing and some example test cases for each area. For each area it would be useful to:

  1. better flesh out what we are trying to test, what is not in scope and generate a(n initial) list of tests to implement
  2. research and propose an appropriate test paradigm for the area which prioritises readability and maintainability of the tests
  3. given 1 and 2, design appropriate test suite

Test areas:

  • Expected error cases
    • Specification defined error cases (need to extract from the specification)
    • The known attacks on package managers (define test scenarios for each attack)
    • Python-tuf implementation error cases (review new code and identify error sequences which are testable)
  • Specific sequences the client should be able to handle – different types of updates sequences that may exist in a repository and which the updater should be able to handle
    • Key rotations
    • Recovery from fast-forward attack
    • Client that has been offline for N expirations of root metadata
    • Air-gapped client outside of timestamp expiration window
    • Some of the previously reported security issues?
    • etc
  • Configuration permutations
    • Ensure we test all the relevant sequences of events with and without consistent snapshots
    • Identify permutations of default config to test (where they are not already adequately tested by the unit tests)
  • Table testing for a set of defined tests with different parameters, i.e.
    • target paths (input to get_one_valid_targetinfo())
    • root rotations
    • different complexity delegation trees – both the client and repository side of the delegation tree operations
      • idea: describe delegations in Graphviz dot format?
@joshuagl
Copy link
Member Author

It probably makes sense to split each test area into a separate issue where the scope, actual tests and proposed test architecture can be defined.

@jku
Copy link
Member

jku commented Sep 15, 2021

Thanks for writing this down. Edited the list to add target paths to table testing section

@sechkova sechkova added the backlog Issues to address with priority for current development goals label Sep 15, 2021
@sechkova sechkova self-assigned this Sep 15, 2021
@sechkova sechkova added this to the Sprint 8 milestone Sep 15, 2021
@jku
Copy link
Member

jku commented Sep 28, 2021

I'm looking at legacy updater tests currently: I'll try to document the actual tests they contain (at least the ones that still apply) and take notes on what useful approaches they might use.

@sechkova
Copy link
Contributor

sechkova commented Oct 6, 2021

After some iterations over the potential test areas and topics, we've narrowed down several bigger ones that we want to start the testing with and keep refining and improving further.
Since tracking of all the tests we want to implement is hard in a GitHub issue, there is a WIP Client test scenarios spread sheet that we mean to constantly update to mark progress.

A preview of the next steps:

  1. Renaming to distinguish old vs new client tests and linting: sync with tests: Start linting tests #1582

  2. Test areas
    2.1 Detailed client workflow
    - Tests scenarios from specification (Can we mark these to be our conformance tests?)
    - ngclient testing: client workflow sequences #1606
    - ngtests: Add tests for delegated roles metadata update #1641
    - ngtests: add tests for fetching targets #1642
    - Specific sequences the client should be able to handle go here
    - Testing specific sequences the client should be able to handle  #1747
    - ngclient: test fast-forward recovery #1713
    - Extracted useful scenarios from legacy tests
    - Specific python-tuf sequences (for ex. loading and validating local metadata)
    - RepositorySimulator based
    2.2 Key rotations tests
    - tests: Updater key rotation tests #1607
    - tests: ngclient non-root key rotations #1647
    - They are part of 2.1 but since there are so many possible variations, it feels like they deserve a separate category
    - Table testing, RepositorySimulator
    2.3 Regression tests
    - Known bugs and CVEs
    2.4 Complex delegation graphs
    - ngtests: test complex delegation graphs #1643
    2.5 Input validation tests for Updater API
    - ngtests: add input validation tests for Updater API #1644
    2.6 Configurations permutation (unless they fit into another category)
    - ngtests: add tests toggling consistent snapshot #1667
    - ngclient: test data length limits #1730
    2.7 Known attacks is a bit vague but should be given some thought
    - ngtests: ensure tests cover known attacks on package managers #1640

  3. Testing tools
    3.1 RepositorySimulator
    - Use it, improve it, enhance it
    - Extend with "spying" capabilities or use the "wrapping" of the unittest mock module
    - RepositorySimulator: add support for serving invalid metadata #1615
    - RepositorySimulator: implement non-consistent snapshot  #1637
    - RepositorySimulator: add support for delegated hash bins #1639
    - RepositorySimulator: add trackers/counters for downloaded files #1682
    3.2 Evaluate how expensive is testing with vs without HTTP server
    - Revise test_updater_ng.py #1748
    3.3 Any useful hand-made decorators (see run_sub_tests_with_dataset)

@jku
Copy link
Member

jku commented Oct 6, 2021

3.1 RepositorySimulator

  • Extend with "spying" capabilities or use the "wrapping" of the unittest mock module

I tried this a bit and it's fairly easy to add things like this:

# assert that only downloads during refresh are root and timestamp
with self.sim.assert_metadata_fetches({'root': 1, 'timestamp': 1}):
    updater.refresh()

it's ugly enough that we likely don't want to do that in every test -- but we could for example write one slightly more complex test with asserts like this to ensure we download exactly what we expect to download.

I think your mock wrap suggestion will be good for ensuring we only read/write the local files we expect to read/write -- similar approach of only testing it in a specific test (and not in every test) probably works.

@jku
Copy link
Member

jku commented Oct 6, 2021

2. 2.2 Key rotations tests
- They are part of 2.1 but since there are so many possible variations, it feels like they deserve a separate category
- Table testing, RepositorySimulator

I have some ideas here (and have also read through all of the relevant legacy tests recently). I'll try to have a go at this within the next days if no-one else beats me to it

@jku
Copy link
Member

jku commented Oct 7, 2021

A new set of tests for your list:

Expiry tests

  • No expired metadata should be accepted as final metadata (as in, loading the "next" metadata should fail)
  • Some metadata (root, timestamp, snapshot) should be loaded as interim metadata even if expired (and should be stored in local metadata cache, see next item)
  • In particular root metadata expiry can happen both as local and as remote metadata -- these are both fine. Still, an expired root should not be accepted as final metadata (as in, loading timestamp should fail)

Some of these may already be tested by TrustedMetadataSet but I tried to list every case...

@MVrachev
Copy link
Collaborator

In pr #1605 we are adding a new configuration option for the RepositorySimulator called compute_metafile_hashes_length which will compute the hashes and length for all MetaFiles defined in timestamp.snapshot_meta and snapshot.meta.
We should add unit tests including this configuration option as well.

@sechkova
Copy link
Contributor

Notes after playing with code a little:

  1. In-memory generation of test files ✔️
    • use RepositorySimulator
  2. Implement "in-memory" fetch replacing the need of setting up a file server ✔️
    • use RepositorySimulator
  3. Fully detach updater from the file system ❌
    • Possible when testing small internal functionality, then built-ins like open can be mocked using the standard unittest mock module.
    • For testing Updater as a whole component, this becomes more complex. Internally updater performs many file reads and writes, writing files to the file system is part of its purpose. We need something similar to a fake file system which is not impossible but given that there are no indicators for heavy file reads/writes slowing tests, lets not focus on this one yet.
  4. "Spying" with unittest mock is easy and helpful when we need to assert that Updater or ReposiotrySimulator have performed any internal call ✔️
        with mock.patch.object(
            simulator, "_fetch_metadata", wraps=simulator._fetch_metadata
        ) as wrapped_fetch_metadata:

        wrapped_fetch_metadata.assert_called_with(...)

"wraps" will pass the call through to the wrapped object (returning the real result).

@sechkova sechkova modified the milestones: Sprint 9, Sprint 14 Dec 8, 2021
@sechkova
Copy link
Contributor

The main categories of tests were identified and implemented (implementation is in progress), see #1579 (comment)
For the tests themselves, here are some guidelines we identified as good to follow:

  • keep the tests short, test only one thing per test case
  • use RepositorySimulator for serving test files in-memory and speeding up the tests
  • use table testing and the unittest SubTest or the custom decorator @utils.run_sub_tests_with_dataset
  • negative test cases: use asserts to confirm exceptions are raised and the files on disk are the expected
  • positive test cases, depending on the test logic:
    • use asserts to confirm existence of files on disk, correct content, version, etc.
    • use unittest mock as a "spy" to check if updater tried to load the files from cache as expected (@patch.object(builtins, "open", wraps=builtins.open)
    • use RepositorySimulator.fetch_trackers to check if updater tried to download files are expected

As an example, see the newly implemented files:
https://github.com/theupdateframework/python-tuf/blob/develop/tests/test_updater_top_level_update.py
https://github.com/theupdateframework/python-tuf/blob/develop/tests/test_updater_key_rotations.py
https://github.com/theupdateframework/python-tuf/blob/develop/tests/test_updater_consistent_snapshot.py
https://github.com/theupdateframework/python-tuf/blob/develop/tests/test_updater_delegation_graphs.py
https://github.com/theupdateframework/python-tuf/blob/develop/tests/test_updater_fetch_target.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Issues to address with priority for current development goals testing
Projects
None yet
Development

No branches or pull requests

4 participants