[BREAKING] [MAINT]: remove rampart_sinks fixture and deprecation warnings - #175
Open
Nina Chikanov (nina-msft) wants to merge 3 commits into
Open
[BREAKING] [MAINT]: remove rampart_sinks fixture and deprecation warnings#175Nina Chikanov (nina-msft) wants to merge 3 commits into
Nina Chikanov (nina-msft) wants to merge 3 commits into
Conversation
…arnings Formal deprecation is unnecessary this early in the project, so remove the deprecated rampart_sinks fixture path entirely and make the pytest_rampart_sinks hook the only way to register report sinks. - Delete rampart/common/deprecation.py and the emit_deprecation_warning helper - Remove the _rampart_sink_bootstrap fixture and _has_sink_hook_impl guard from the pytest plugin; resolve sinks via the hook only - Remove discover_sinks_from_conftest and its fixture-unwrap helpers from _xdist - Guard --collect-only in pytest_sessionfinish so collection never emits reports (previously an implicit side effect of fixture timing) - Migrate the xdist aggregation test conftest to the hook and drop deprecation tests - Update docs to describe hook-only sink registration Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Nina Chikanov (nina-msft)
changed the title
| @@ -1,56 +0,0 @@ | |||
| # Copyright (c) Microsoft Corporation. | |||
Contributor
Author
There was a problem hiding this comment.
Not sure if we want to keep this around in case we want to deprecate in the future. Since its only one file, we can easily write this up again once we decide this project needs a formal deprecation strategy
msgraph-sdk is only installed via the onedrive extra, so ty cannot resolve its TYPE_CHECKING import in the default environment. Add an inline ty: ignore[unresolved-import] matching the repo's suppression convention. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The unresolved-import failure was caused by a local environment synced without the optional onedrive extra, not by the source. CI runs uv sync --all-extras, so msgraph-sdk is present and the import resolves; the inline ty: ignore would then be flagged as an unused-ignore-comment and fail CI. Revert to the original import and resolve locally with uv sync --all-extras. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Nina Chikanov (nina-msft)
marked this pull request as ready for review
August 26, 2026 00:21
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Team discussed that formal deprecation cycles are unnecessary this early in the project, so this removes the deprecated
rampart_sinksfixture path entirely rather than keeping it around with warnings. Thepytest_rampart_sinkshook becomes the single, enforced way to register report sinks.Key changes:
rampart/common/deprecation.pyand itsemit_deprecation_warninghelper._rampart_sink_bootstrapautouse fixture and the now-unused_has_sink_hook_implguard from the pytest plugin; sink resolution now goes through the hook only, in both single-process and xdist controller runs.discover_sinks_from_conftestand its fixture-unwrap helpers from_xdist.py.--collect-onlyguard inpytest_sessionfinish. "No reports on collect-only" was previously an accidental side effect of fixture timing; with the hook resolved unconditionally at session finish, it is now enforced directly so collection never overwrites report files.Validation:
ruff check .is clean and the full suite passes (1013 passed, 5 skipped), including the slow subprocess xdist tests.Breaking changes
Yes. Projects that register sinks via a
rampart_sinksfixture will silently stop emitting reports. Migration: define apytest_rampart_sinks(config)hook inconftest.pyreturning the same list of sinks.Checklist
pre-commit run --all-filespasses