Skip to content

[FEAT]: Deprecate the rampart_sinks fixture in favor of the pytest_rampart_sinks hook#105

Open
nina-msft wants to merge 6 commits into
microsoft:mainfrom
nina-msft:dev/nina-msft/deprecate-sinks-fixture
Open

[FEAT]: Deprecate the rampart_sinks fixture in favor of the pytest_rampart_sinks hook#105
nina-msft wants to merge 6 commits into
microsoft:mainfrom
nina-msft:dev/nina-msft/deprecate-sinks-fixture

Conversation

@nina-msft

@nina-msft nina-msft commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #73. Now that the pytest_rampart_sinks hook is the supported
sink-registration API, this PR begins deprecating the legacy rampart_sinks
fixture.

Resolving the fixture now emits a DeprecationWarning (removal target: 0.3.0)
in both fixture-consumption paths:

  • Single-process_rampart_sink_bootstrap, after the fixture resolves.
  • xdist controller_resolve_sink_candidate, when it unwraps a genuine
    @pytest.fixture-form rampart_sinks (not module-level lists or plain
    callables).

The hook stays authoritative and the fixture remains a single-process fallback,
so nothing breaks for existing users — they just get a migration nudge.

Changes

  • New rampart/common/deprecation.pyemit_deprecation_warning(...),
    mirroring PyRIT's helper (qualified-name resolution + DeprecationWarning at
    stacklevel=3).
  • Wire it into the plugin.py and _xdist.py fixture-resolution paths.
  • Docs: deprecation admonitions in usage/pytest-integration.md and
    usage/xdist.md.
  • Tests: tests/unit/common/test_deprecation.py (unit) and a
    TestSinkFixtureDeprecation class in the slow aggregation suite that drives
    real subprocess pytest runs via pytester — single-process warn,
    xdist-controller warn, and list-form no-warn — replacing the previous
    MagicMock-based warn/no-warn tests.

Breaking changes

None. The fixture continues to work as a single-process fallback; it only emits
a DeprecationWarning now.

Validation

  • uv run ruff check . → all checks pass; ruff format --check clean.
  • uv run ty check → all checks passed.
  • uv run pytest tests/unit/common/test_deprecation.py tests/unit/pytest_plugin/test_xdist.py tests/unit/pytest_plugin/test_plugin.py → 145 passed.
  • uv run pytest tests/unit/pytest_plugin/test_xdist_aggregation.py -m slow → 16 passed (includes the pytester-based TestSinkFixtureDeprecation class).

Notes

  • removed_in="0.3.0" is the removal milestone for the fixture — deprecated in
    the 0.1 line, kept through 0.2, removed in 0.3 (two minor versions, per
    docs/contributing/release-process.md).
  • Implements the first of the post-merge deprecation follow-ups raised by
    @bashirpartovi.

Checklist

  • pre-commit run --all-files passes
  • Tests added or updated for changes
  • Documentation updated

@nina-msft
nina-msft marked this pull request as ready for review July 7, 2026 23:17
@nina-msft
nina-msft requested a review from a team July 7, 2026 23:17
@nina-msft
nina-msft force-pushed the dev/nina-msft/deprecate-sinks-fixture branch from 6de45a8 to 1c373d9 Compare July 7, 2026 23:35
Comment thread docs/usage/pytest-integration.md

@spencrr spencrr 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.

Looks good! First quick pass:

  • print_deprecation_message -> log deprecation warning maybe?
  • teats have a lot of mocks -> let’s use pytester and capture warnings that way?

@nina-msft
nina-msft force-pushed the dev/nina-msft/deprecate-sinks-fixture branch from 1c373d9 to cd89f7b Compare July 8, 2026 00:10
@nina-msft

Copy link
Copy Markdown
Contributor Author

@spencrr I just updated the test name to be emit_deprecation_warning before seeing your comment so that is already addressed :D I'll do another pass through the tests shortly!

The pytest_rampart_sinks hook is the supported sink registration API. Resolving the legacy rampart_sinks fixture - in the single-process bootstrap and in controller-side discovery - now emits a DeprecationWarning pointing users to the hook, scheduled for removal in 1.0.0.

Adds rampart/common/deprecation.py (mirrors PyRIT's print_deprecation_message), wires it into both fixture-consumption paths, documents the deprecation in the pytest-integration and xdist guides, and adds unit + pytester coverage.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nina-msft
nina-msft force-pushed the dev/nina-msft/deprecate-sinks-fixture branch from cd89f7b to 3ea79a3 Compare July 8, 2026 00:52
Comment thread tests/unit/pytest_plugin/test_xdist_aggregation.py Outdated
@nina-msft
nina-msft enabled auto-merge (squash) July 10, 2026 23:52

@bashirpartovi bashirpartovi 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.

This is great @nina-msft . However, there are a few places in the documentation that need to be updated:

  • Quickstart doc, we need to update Step 4:
  • concepts/overview.md describes the feature as "Report sinks: Structured output via the rampart_sinks fixture" see pytest-integration
  • ci-integration.md shows a full @pytest.fixture … def rampart_sinks() code sample as the main example (it does have a downstream !!! tip pointing at the hook for xdist, so it's the least bad of the 3, but the headline example is still the fixture).

@nina-msft
nina-msft disabled auto-merge July 15, 2026 17:13
nina-msft and others added 5 commits July 17, 2026 11:55
Update the five docs that still presented the deprecated rampart_sinks fixture as the primary path (quickstart Step 4, concepts overview, ci-integration headline example, usage index nav, results-and-reporting wiring) to lead with the pytest_rampart_sinks hook and frame the fixture as a deprecated fallback. Fixes anchor links to #pytest_rampart_sinks-hook and drops the now-unused pytest/ReportSink imports from the CI sample.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 80e88044-7224-4c36-b9ac-3873c4c3d2ae
Resolve conflict in tests/unit/pytest_plugin/test_xdist_aggregation.py by
adopting upstream's configured_pytester fixture (asyncio ini isolation) and
layering this branch's fixture-deprecation additions on top:
- _LIST_CONFTEST module-level list-form sinks conftest
- TestSinkFixtureDeprecation covering single-process warn, xdist controller
  warn, and list-form no-warn cases

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 80e88044-7224-4c36-b9ac-3873c4c3d2ae
The merged-in ruff DOC ruleset flags a ``Returns: None`` section on a
``-> None`` function (docstring-extraneous-returns). Remove it so
``ruff check`` passes.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 80e88044-7224-4c36-b9ac-3873c4c3d2ae
Deprecating the `rampart_sinks` fixture (not the module-level list form)
means the list form must stay silent. That contract was guarded only by
the slow `pytester` subprocess test
`test_controller_list_form_does_not_warn_under_xdist`, whose discovery and
report-count assertions already overlap `test_finds_list_rampart_sinks` and
`test_xdist_emits_single_consolidated_report`.

Move the one unique assertion (list form emits no deprecation warning) into
fast, in-process tests in `TestSinkDeprecationWarning`, adding a symmetric
"fixture form warns" assertion that no unit test previously covered. Drop the
subprocess test and its now-unused `_LIST_CONFTEST`.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 80e88044-7224-4c36-b9ac-3873c4c3d2ae
The slow xdist aggregation tests re-verify verdict/threshold/rendering
logic that is already fully unit-tested in test_plugin.py. Trim the
redundant subprocess permutations while keeping the genuine integration
guards, and backfill the two verdict cases that were only covered
end-to-end so no coverage is lost.

- Fold test_population_statistics_over_full_set into
  test_xdist_emits_single_consolidated_report (one fewer -n 2 run).
- Drop the three loadgroup trial-group verdict permutations; keep the
  safety-critical fails_when_any_unsafe_under_load end-to-end guard.
- Add below-threshold-FAIL and all-safe-PASS model tests to
  TestSessionRecordTrialGroup and a PASSED-line test to
  TestTrialGroupRendering.

Removes ~4 slow subprocess runs (~90s each) with equivalent fast
in-process coverage.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 80e88044-7224-4c36-b9ac-3873c4c3d2ae
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