Add first graph and target search test

[AdamKorcz]

Adds the first graph and target search test as mentioned in #101.

This adds the get_targetinfo client API for both clients too. This might need more work for tests that actually use the output. The test in this PR does not use the output.

Interestingly, go-tuf fails in an indeterministic manner from this test; Different test cases will fail each time you run the test. I have tried removing all the testcases except for the "max-number-of-delegations" one, and this still fails sometimes. @rdimitrov could you check this? You can test it by cloning my branch and running make test-go-tuf. I have left some print statements for debugging assistance.

[AdamKorcz]

I am putting this in draft. I am not getting the same failures locally. Will need to debug this. Please wait with the review.

[segiddins]

I think it might be cleaner instead to have the get_targetinfo entry point write the target info to a provided path, rather than relying on STDOUT having nothing else printed?

[AdamKorcz]

@jku I am unable to reproduce the same outcome as the CI for python-tuf locally. Could you check if you can reproduce it?

[jku]

I'll have a look at the failures but as a first comment:
Let's not expand the CLI if at all possible -- I'm not sure we can assume that all libraries even expose API like this. Can we not use download command to achieve similar testable situations? I especially don't want any kind of structured output requirements for the CLI: it is just a pain for client developers

[jku]

The issue at least with python-tuf can be reproduced with --repository-dump-dir /tmp/dumps. There's two issues that I can see :

1. The code added to clients/python-tuf assumes get_targetinfo() always returns TargetFile: this is untrue (maybe we should enable linting in clients, mypy would have complained about this: lint: Add the python client to linted files #118)

2. The test also fails because the dumping code invoked by --repository-dump-dir uses fetch_metadata() which ends up breaking FetchTracker.

If you use the already existing metadata_statistics and artifact_statistics instead of FetchTracker this should not be an issue (because those are bumped in fetch() that is only called by the http handler, not other code)

[jku]

Is this the flaky failure for go-tuf:

FAILED tuf_conformance/test_updater_delegation_graphs.py::test_graph_traversal[two-level-delegations] - AssertionError: assert [('B', 1), ('C', 1), ('A', 1)] == [('A', 1), ('B', 1), ('C', 1)]

very interesting if it is really flaky... It could of course be a bug in our request tracking but I think that is pretty simple and should not lead to ordering issues

[AdamKorcz]

Is this the flaky failure for go-tuf:

FAILED tuf_conformance/test_updater_delegation_graphs.py::test_graph_traversal[two-level-delegations] - AssertionError: assert [('B', 1), ('C', 1), ('A', 1)] == [('A', 1), ('B', 1), ('C', 1)]

very interesting if it is really flaky... It could of course be a bug in our request tracking but I think that is pretty simple and should not lead to ordering issues

In this run another test - three-level-terminating-ignores-all-but-roles-descendants - fails. I believe this is similar to the flakiness I described in the initial PR message.

[jku]

I know you said don't review yet... I'm doing that anyway since this is kind of blocking the continuation like #140.

All comments are minor -- the only issue preventing merging is go-tuf flakiness. I don't have great ideas for dealing with that. We would have to either

• support skipping in the action API like we do expected failures

• or add a hard coded special case in the test until go-tuf is fixed

    if ("clients/go-tuf/go-tuf" in client._cmd):
       pytest.skip("skip for flakiness")
  

maybe the hard coded skip (and filing an issue to remind us to remove it) makes sense so we can move and merge this?

[AdamKorcz]

For info: The cyclic-graph test fails because there are not enough signers.

[AdamKorcz]

if ("clients/go-tuf/go-tuf" in client._cmd):
pytest.skip("skip for flakiness")

Added in 990c5e9

[jku]

Thanks, looks good.

• The 150 lines of setup data is not amazing but I don't see meaningful improvements (and some of it, like artifact support, is for future tests)
• left one comment on return values