Skip to content

test: declare ops-scenario and ops-tracing test dependencies - #2891

Open
tonyandrewmeyer wants to merge 4 commits into
canonical:mainfrom
tonyandrewmeyer:subproject-test-deps
Open

tonyandrewmeyer wants to merge 4 commits into
canonical:mainfrom
tonyandrewmeyer:subproject-test-deps

Conversation

@tonyandrewmeyer

@tonyandrewmeyer tonyandrewmeyer commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator

ops-scenario's tests depend on ops-tracing, but ops-scenario doesn't declare it, so the tests only work when installed through the repository root's unit group. This adds a unit group to ops-scenario's pyproject.toml, and has ops-tracing's unit group name ops-scenario directly rather than ops[testing] (ops is already a runtime dependency).

  • test_trace_data is skipped when ops-tracing isn't installed, since it's an optional dependency of ops-scenario.
  • The benchmark tests are skipped when pytest-benchmark isn't installed, since they need its benchmark fixture.

Running testing/tests on its own also gave 17 errors in test_secrets.py, because the secrets test charm's metadata path was resolved relative to pytest's rootdir. The fixture now resolves it relative to the conftest file instead, so the tests pass however pytest is invoked.

This is useful when an automated system is figuring out how to run the tests.

ops-scenario's tests need ops-tracing, but ops-scenario didn't declare
it. It was only in the repository root's unit group. Add a unit group
to ops-scenario so its tests can run from its own pyproject.toml.

ops-tracing's unit group asked for `ops[testing]`. Name ops-scenario
directly instead, since that's the package the tests use (ops is
already a runtime dependency).

ops-tracing is an optional dependency of ops-scenario, so skip
test_trace_data when it isn't installed rather than failing. Also skip
the benchmark tests when pytest-benchmark isn't installed, since they
need its fixture.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tonyandrewmeyer

Copy link
Copy Markdown
Collaborator Author

The CI failures are due to the current snap store outage, I believe, so should be resolved when that's fixed.

@dwilding

Copy link
Copy Markdown
Contributor

I haven't started reviewing yet. But:

We might want to fix this later, though.

My agent found exactly this problem yesterday - I assume because we're both asking agents to run tests within workshops. FWIW, here's my agent's report.


Issue: testing/tests/test_e2e/conftest.py:25 resolves the secrets test charm via pytestconfig.rootpath, but the repo has no pytest config file, so rootdir floats to the common ancestor of the pytest arguments — any invocation naming a testing/tests path (manual or tox -e unit -- <path>) makes rootdir /project/testing and 17 tests fail with FileNotFoundError.

Suggested fix:

  • Add [tool.pytest.ini_options] with testpaths = ["test", "testing/tests"] to the root pyproject.toml, which anchors rootdir at the repo root for every invocation.
  • Change conftest.py:25 to resolve the path against the ops package (Path(ops.__file__).parents[1]), removing the rootdir dependency entirely.
  • Add a collect_ignore for testing/tests/benchmark/ when pytest-benchmark is absent, so targeted runs don't error on the 10 benchmark tests.

@dwilding dwilding left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The uv.lock revision has reverted to 3 (from 5), which I don't think matters. But do you want to regenerate it with a newer uv version, to avoid churn in a future PR?

Comment thread testing/tests/benchmark/test_testing.py Resolved
tonyandrewmeyer and others added 2 commits November 12, 2026 18:26
Resolve the secrets test charm's metadata path relative to the
conftest file rather than pytest's rootdir, so the tests pass when
testing/tests is run on its own (the rootdir is then testing/).

Move the pytest-benchmark comment onto the importorskip line, and
regenerate uv.lock from main with a newer uv so it stays at lockfile
revision 5.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tonyandrewmeyer

Copy link
Copy Markdown
Collaborator Author

The uv.lock revision has reverted to 3 (from 5), which I don't think matters. But do you want to regenerate it with a newer uv version, to avoid churn in a future PR?

I've made the change minimal here. I'm not sure what to do about these in general. It's annoying to have churn, maybe we need to try to use a consistent uv somehow? One issue at the moment is that the uv snap is quite behind the actual uv version.

@tromai

tromai commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

ops-scenario's tests depend on ops-tracing, but ops-scenario doesn't declare it, so the tests only work when installed through the repository root's unit group.

I'm curious how I could reproduce this error. Was it running tox -e unit with some flags, or something else ? Or maybe this sentence is referring to the 17 errors in test_secrets.py ?

I could reproduced the 17 errors in test_secrets.py, so the PR's fix is good:

  • uv sync --all-groups in the repo's root
  • cd into testing
  • pytest ./

@tromai tromai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Leaving a small comment on reproducing the fix.

Add testing/tox.ini, matching tracing's, so the testing unit dependency
group can be installed and run on its own with `cd testing && tox -e
unit`. CI already runs testing/tests through the root unit
environment.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

3 participants