Skip to content

Fix release package test dependencies#72

Merged
msureshkumar88 merged 1 commit intomainfrom
fix/release-workflow-dev-test-deps
May 1, 2026
Merged

Fix release package test dependencies#72
msureshkumar88 merged 1 commit intomainfrom
fix/release-workflow-dev-test-deps

Conversation

@lucarlig
Copy link
Copy Markdown
Collaborator

@lucarlig lucarlig commented May 1, 2026

Summary

Fixes the Rust Python package release workflow isolated package tests by installing each plugin's declared dev dependency group into the temporary test virtualenv before installing the built wheel or sdist. PyYAML remains explicitly installed because the shared release test helper imports yaml.

Also makes PR release validation target actual plugin version bumps: when a plugin Cargo.toml version changes, CI now runs the reusable release workflow for that plugin's computed release tag, for example rate-limiter-v0.0.2.

Root Cause

The secrets-detection-v0.2.1 release failed in build-sdist and wheel jobs because the isolated test step installed only pytest pytest-asyncio PyYAML. The checked-in secrets_detection test helpers import pydantic, which is declared in the plugin's dev dependency group but was not installed by the release workflow test environment.

The previous PR release-validation check also used a fixed sample tag, so it did not necessarily validate the plugin being version-bumped in a PR.

Impact

Release package tests now use the per-plugin test dependencies already declared in pyproject.toml, avoiding most hard-coded shared test dependency drift while still preserving PyYAML for shared helper compatibility.

Version-bump PRs now run release-validation against the bumped plugin package before the release tag is created.

Regression Coverage

The CI-run plugin catalog suite now asserts that:

  • release artifact test steps install the plugin dev dependency group plus PyYAML
  • the workflow does not regress to the old hard-coded pytest pytest-asyncio PyYAML list
  • ci-selection emits release-validation tags for plugin version bumps
  • the CI release-validation job uses those computed tags as a matrix

Verification

  • verified plugins/tests/plugin_hooks.py imports yaml
  • uv pip install --python ... --group dev --dry-run includes pydantic
  • local sdist isolated package test: 74 passed
  • local wheel isolated package test: 74 passed
  • python3 -m unittest tests/test_plugin_catalog.py
  • python3 tools/plugin_catalog.py ci-selection . diff origin/main HEAD | python3 tools/validate_ci_selection.py
  • git diff --check

Note

The failed tag run checked out the old workflow from tag secrets-detection-v0.2.1. After this merges, rerun via workflow_dispatch from main with that tag.

@lucarlig lucarlig force-pushed the fix/release-workflow-dev-test-deps branch 2 times, most recently from 6e56de4 to 2a10859 Compare May 1, 2026 11:32
@lucarlig lucarlig marked this pull request as ready for review May 1, 2026 11:39
@lucarlig lucarlig force-pushed the fix/release-workflow-dev-test-deps branch from 2a10859 to 89338c7 Compare May 1, 2026 12:15
Copy link
Copy Markdown
Collaborator

@msureshkumar88 msureshkumar88 left a comment

Choose a reason for hiding this comment

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

Review: Fix release package test dependencies

Overall: correct fix, one concern worth addressing.

What the PR does

Replaces hard-coded in the isolated test venv step with uv pip install --group dev, so each plugin's declared dev dependencies are used instead of a shared static list. Root cause was pydantic missing for secrets_detection tests.

Good

  • working-directory: ${{ needs.resolve.outputs.plugin_path }} is already set on the test step, so uv pip install --group dev reads the correct plugin's pyproject.toml. No --project flag needed.
  • All 6 Rust Python packages already declare pytest>=8.0 and pytest-asyncio>=0.23 in their dev groups — no regression risk there.
  • Test assertions in test_plugin_catalog.py match the existing string-check pattern in that file.

Concern: PyYAML silently dropped for 5 of 6 plugins

The old hard-coded install included PyYAML. Only secrets_detection carries PyYAML>=6.0 in its dev group. The other 5 plugins (encoded_exfil_detection, pii_filter, rate_limiter, retry_with_backoff, url_reputation) do not.

If the shared test helpers copied into the isolated venv — plugin_hooks.py or conftest.py — import yaml, those five plugin release tests will fail after this merges.

Please verify plugins/tests/plugin_hooks.py and plugins/tests/conftest.py do not import yaml. If they do, either:

  • Add PyYAML to each affected plugin's dev group, or
  • Add it back as an explicit install alongside --group dev:
    uv pip install --python "${venv_python}" --group dev PyYAML
    "${venv_python}" -m pip install dist/*.whl

The assertNotIn('pytest pytest-asyncio PyYAML', workflow) test assertion passes either way, so it won't catch this regression.

Copy link
Copy Markdown
Collaborator

@msureshkumar88 msureshkumar88 left a comment

Choose a reason for hiding this comment

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

Review: Fix release package test dependencies

Overall: correct fix, one concern worth addressing.

What the PR does

Replaces hard-coded pytest pytest-asyncio PyYAML in the isolated test venv step with uv pip install --group dev, so each plugin's declared dev dependencies are used instead of a shared static list. Root cause was pydantic missing for secrets_detection tests.

Good

  • working-directory: ${{ needs.resolve.outputs.plugin_path }} is already set on the test step, so uv pip install --group dev reads the correct plugin's pyproject.toml. No --project flag needed.
  • All 6 Rust Python packages already declare pytest>=8.0 and pytest-asyncio>=0.23 in their dev groups — no regression risk there.
  • Test assertions in test_plugin_catalog.py match the existing string-check pattern in that file.

Concern: PyYAML silently dropped for 5 of 6 plugins

The old hard-coded install included PyYAML. Only secrets_detection carries PyYAML>=6.0 in its dev group. The other 5 plugins (encoded_exfil_detection, pii_filter, rate_limiter, retry_with_backoff, url_reputation) do not.

If the shared test helpers copied into the isolated venv — plugin_hooks.py or conftest.py — import yaml, those five plugin release tests will fail after this merges.

Please verify plugins/tests/plugin_hooks.py and plugins/tests/conftest.py do not import yaml. If they do, either:

  • Add PyYAML to each affected plugin's dev group, or
  • Add it back as an explicit install alongside --group dev:
    uv pip install --python "${venv_python}" --group dev PyYAML
    "${venv_python}" -m pip install dist/*.whl

The assertNotIn('pytest pytest-asyncio PyYAML', workflow) test assertion passes either way, so it won't catch this regression.

Signed-off-by: lucarlig <luca.carlig@ibm.com>
@lucarlig lucarlig force-pushed the fix/release-workflow-dev-test-deps branch from 89338c7 to 6792807 Compare May 1, 2026 12:28
@lucarlig
Copy link
Copy Markdown
Collaborator Author

lucarlig commented May 1, 2026

@msureshkumar88 addressed your PyYAML concern.

I verified plugins/tests/plugin_hooks.py imports yaml, so the release workflow now preserves that shared helper dependency explicitly while still installing each plugin dev group:

uv pip install --python "${venv_python}" --group dev PyYAML

This is applied in both wheel and sdist isolated test steps. I also updated the catalog regression test to require that exact install pattern, so we do not silently drop PyYAML again.

Local verification run:

  • python3 -m unittest tests/test_plugin_catalog.py
  • python3 tools/plugin_catalog.py ci-selection . diff origin/main HEAD | python3 tools/validate_ci_selection.py
  • git diff --check

Copy link
Copy Markdown
Collaborator

@msureshkumar88 msureshkumar88 left a comment

Choose a reason for hiding this comment

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

Review follow-up

Concern from previous review (PyYAML silently dropped for 5/6 plugins) is addressed — PyYAML is now explicitly passed alongside --group dev, and the test assertion verifies it stays.

One new item to confirm before merge:

uv pip install --group dev requires uv ≥ 0.4.15. If the CI runner pins an older uv version, this will fail. Worth verifying the uv version available in the release workflow runner.

Also note: dev deps and wheel are now installed in two separate commands (uv first, then pip). This differs from the original single-pip call but should be harmless — pip resolves wheel conflicts against already-installed dev deps.

Pending CI checks (coverage, release-validation/preflight, ubuntu/secrets_detection, ubuntu/pii_filter) need to complete before merge.

Overall the fix is correct. LGTM once uv version is confirmed and CI is green.

@lucarlig
Copy link
Copy Markdown
Collaborator Author

lucarlig commented May 1, 2026

@msureshkumar88 confirmed the CI runner version from the completed release-validation logs.

The release workflow installs uv==0.9.30 explicitly before the package build/test steps:

python -m pip install uv==0.9.30 maturin==1.12.6
Successfully installed maturin-1.12.6 uv-0.9.30

That is well above the uv pip install --group dev minimum you called out (uv >= 0.4.15). The same completed release-validation logs also show the exact command running successfully:

uv pip install --python "${venv_python}" --group dev PyYAML

Current PR check summary is green: 34 passing, 3 skipped, 0 pending, 0 failed.

Copy link
Copy Markdown
Collaborator

@msureshkumar88 msureshkumar88 left a comment

Choose a reason for hiding this comment

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

Both concerns from my previous review are resolved:

  • PyYAML: explicitly installed alongside --group dev in both wheel and sdist steps; catalog test asserts it stays.
  • uv version: release workflow pins uv==0.9.30, well above the --group dev minimum (0.4.15). Confirmed in completed release-validation logs.
  • CI green: 34 passing, 3 skipped, 0 failed.

Any further hardening (e.g. adding PyYAML to all 5 remaining plugin dev groups so the explicit pin can eventually be dropped) is a future enhancement, not a blocker.

LGTM. Approve to merge.

@msureshkumar88 msureshkumar88 merged commit 7460a85 into main May 1, 2026
37 checks passed
@msureshkumar88 msureshkumar88 deleted the fix/release-workflow-dev-test-deps branch May 1, 2026 13:57
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