Skip to content

Commit

Permalink
Improve the "expected failures" experience (#178)
Browse files Browse the repository at this point in the history
If we add more tests that we consider more or less optional features,
we need a way for clients to manage them.

xfails seems like it should work for that: Make it possible to
write the expected failures to a file next to the client-under-test
executable. This should make managing a longer xfail list reasonable.

This also means we don't need to repeat the xfails for go-tuf in two
places.

Signed-off-by: Jussi Kukkonen <[email protected]>
  • Loading branch information
jku authored Aug 29, 2024
1 parent 12c0829 commit 733adb0
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 21 deletions.
1 change: 0 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,3 @@ jobs:
uses: ./
with:
entrypoint: "clients/go-tuf/go-tuf"
expected-failures: "test_keytype_and_scheme[rsa/rsassa-pss-sha256]"
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ PHONY: test-go-tuf
test-go-tuf: dev build-go-tuf faketime
./env/bin/pytest tuf_conformance \
--entrypoint "./clients/go-tuf/go-tuf" \
--expected-failures "test_keytype_and_scheme[rsa/rsassa-pss-sha256]" \
--repository-dump-dir $(DUMP_DIR)
@echo Repository dump in $(DUMP_DIR)
PHONY: build-go-tuf
Expand Down
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ There are two required steps:
entrypoint: path/to/my/test/executable
```
### Expected failures
The test suite also contains tests that are not strictly speaking specification requirements (such as
tests for specific keytype or hash algorithm suport). Clients can mark tests as "expected failures"
if they do not intend to support this specific feature.
The tests that are expected to fail can be listed in `<entrypoint>.xfails` file. In the previous
workflow example the xfails file would be `path/to/my/test/executable.xfails`

## Development

Expand Down
6 changes: 0 additions & 6 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ inputs:
but if you call this action in a job matrix, make sure each call gets a unique name"
default: ""
required: false
expected-failures:
description: "Optional list test names expected to fail"
default: ""
required: false

runs:
using: "composite"
Expand All @@ -36,7 +32,6 @@ runs:
id: tuf-conformance
env:
ENTRYPOINT: ${{ inputs.entrypoint }}
EXPECTED_FAILURES: ${{ inputs.expected-failures }}
TEST_LOCATION: ${{ github.action_path }}/tuf_conformance
NAME: ${{ inputs.artifact-name }}
run: |
Expand All @@ -50,7 +45,6 @@ runs:
# run test suite
pytest -v "$TEST_LOCATION" \
--entrypoint "$ENTRYPOINT" \
--expected-failures "$EXPECTED_FAILURES" \
--repository-dump-dir ./test-repositories \
shell: bash

Expand Down
1 change: 1 addition & 0 deletions clients/go-tuf/go-tuf.xfails
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test_keytype_and_scheme[rsa/rsassa-pss-sha256]
26 changes: 15 additions & 11 deletions tuf_conformance/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,6 @@ def pytest_addoption(parser: pytest.Parser) -> None:
required=True,
type=str,
)
parser.addoption(
"--expected-failures",
action="store",
help="Optional space delimited list of test names expected to fail",
required=False,
type=str,
)
parser.addoption(
"--repository-dump-dir",
action="store",
Expand Down Expand Up @@ -90,14 +83,25 @@ def static_client(
return ClientRunner(entrypoint, static_server, request.node.name)


@cache
def read_xfails(pytestconfig: pytest.Config) -> list[str]:
# Find expected failures from .xfails file
xfail_file = f"{pytestconfig.getoption('--entrypoint')}.xfails"
if not os.path.isabs(xfail_file):
xfail_file = os.path.join(pytestconfig.invocation_params.dir, xfail_file)

try:
with open(xfail_file) as f:
return f.read().splitlines()
except FileNotFoundError:
return []


@pytest.fixture(autouse=True)
def conformance_xfail(
pytestconfig: pytest.Config, request: pytest.FixtureRequest
) -> None:
xfail_option = pytestconfig.getoption("--expected-failures")
if xfail_option is None:
return
xfails = read_xfails(pytestconfig)

xfails = xfail_option.split(" ")
if request.node.originalname in xfails or request.node.name in xfails:
request.node.add_marker(pytest.mark.xfail(strict=True))
4 changes: 2 additions & 2 deletions tuf_conformance/test_file_download.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,8 @@ def test_download_with_hash_algorithms(
"""Test support of hash algorithms. The specification does not require any
specific algorithms, and only mentions sha256.
Clients can use --expected-failures to mark tests that fail because of algorithms
they do not intend to support."""
Clients can use the .xfail file to mark tests as "expected to fail".
"""
init_data, repo = server.new_test(client.test_name)

# Create a legitimate test artifact
Expand Down

0 comments on commit 733adb0

Please sign in to comment.