Skip to content

test: add verification for spdx licenses#1225

Merged
fege merged 2 commits intoopendatahub-io:mainfrom
fege:spdx_tests
Mar 16, 2026
Merged

test: add verification for spdx licenses#1225
fege merged 2 commits intoopendatahub-io:mainfrom
fege:spdx_tests

Conversation

@fege
Copy link
Copy Markdown
Contributor

@fege fege commented Mar 16, 2026

Pull Request

Summary

  • Add test for spdx license
  • Remove xfail, the bug is fixed and deployed downstream
  • Update CONSITUTION.md

Related Issues

  • Fixes:
  • JIRA: RHOAIENG-52377

How it has been tested

  • Locally
  • Jenkins

Additional Requirements

  • If this PR introduces a new test image, did you create a PR to mirror it in disconnected environment?
  • If this PR introduces new marker(s)/adds a new component, was relevant ticket created to update relevant Jenkins job?

Summary by CodeRabbit

  • Documentation

    • Clarified contributor guidance on acceptable ticket references; only publicly accessible resources may be cited in this public repository.
  • Tests

    • Expanded search test coverage with an additional license-filter scenario.
    • Changed a previously xfailed ordering test to be expected to pass.

@fege fege requested review from a team, dbasunag and lugi0 as code owners March 16, 2026 14:25
@github-actions
Copy link
Copy Markdown

The following are automatically added/executed:

  • PR size label.
  • Run pre-commit
  • Run tox
  • Add PR author as the PR assignee
  • Build image based on the PR

Available user actions:

  • To mark a PR as WIP, add /wip in a comment. To remove it from the PR comment /wip cancel to the PR.
  • To block merging of a PR, add /hold in a comment. To un-block merging of PR comment /hold cancel.
  • To mark a PR as approved, add /lgtm in a comment. To remove, add /lgtm cancel.
    lgtm label removed on each new commit push.
  • To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
    verified label removed on each new commit push.
  • To Cherry-pick a merged PR /cherry-pick <target_branch_name> to the PR. If <target_branch_name> is valid,
    and the current PR is merged, a cherry-picked PR would be created and linked to the current PR.
  • To build and push image to quay, add /build-push-pr-image in a comment. This would create an image with tag
    pr-<pr_number> to quay repository. This image tag, however would be deleted on PR merge or close action.
Supported labels

{'/wip', '/cherry-pick', '/hold', '/build-push-pr-image', '/lgtm', '/verified'}

Signed-off-by: fege <fmosca@redhat.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 16, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 9cd3c25b-483b-4fef-b7c6-8ccc8a06a616

📥 Commits

Reviewing files that changed from the base of the PR and between 232d944 and 2ea7e9e.

📒 Files selected for processing (3)
  • CONSTITUTION.md
  • tests/model_registry/mcp_servers/search/test_filtering.py
  • tests/model_registry/mcp_servers/search/test_ordering.py
💤 Files with no reviewable changes (1)
  • tests/model_registry/mcp_servers/search/test_ordering.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • tests/model_registry/mcp_servers/search/test_filtering.py
  • CONSTITUTION.md

📝 Walkthrough

Walkthrough

Updated security guidance in CONSTITUTION.md; added a BSD 3-Clause license filter test case for MCP server search; removed an xfail marker from an ordering test so it now runs as a normal test.

Changes

Cohort / File(s) Summary
Security Policy
CONSTITUTION.md
Added three bullets under "Security Awareness (VII)" allowing links to Jira tickets in PRs/commits only if publicly accessible, and prohibiting references to internal-only or embargoed/security-restricted tickets. Appended rationale noting this is a public repository.
MCP Server Search Tests
tests/model_registry/mcp_servers/search/test_filtering.py
Added a parametrized test case for license='BSD 3-Clause' expecting one match ("file-manager"). Updated test docstring to include TC-API-003/005/009.
MCP Server Ordering Tests
tests/model_registry/mcp_servers/search/test_ordering.py
Removed @pytest.mark.xfail from test_ordering_by_name, making it a regular (expected-pass) test.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding SPDX license verification test. It is concise, specific, and clearly conveys the primary intent of the changeset.
Description check ✅ Passed The description covers all required sections with appropriate content: a summary of changes, related JIRA ticket, and testing checklist. All required template sections are present and substantively filled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

You can get early access to new features in CodeRabbit.

Enable the early_access setting to enable early access features such as new models, tools, and more.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/model_registry/mcp_servers/search/test_filtering.py`:
- Line 27: The test case uses the wrong license string so the filter returns
zero results; update the pytest.param entry in test_filtering.py (the parameter
with id="by_license" and target "file-manager") to use license='BSD-3-Clause'
(hyphens) instead of license='BSD 3-Clause' so it matches the fixture, i.e.,
adjust the pytest.param argument passed to the filtering test to the correct
license format.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 7cc98d31-bde4-42b1-96b6-65251a4bee8f

📥 Commits

Reviewing files that changed from the base of the PR and between 2ac4148 and 232d944.

📒 Files selected for processing (3)
  • CONSTITUTION.md
  • tests/model_registry/mcp_servers/search/test_filtering.py
  • tests/model_registry/mcp_servers/search/test_ordering.py
💤 Files with no reviewable changes (1)
  • tests/model_registry/mcp_servers/search/test_ordering.py

Comment thread tests/model_registry/mcp_servers/search/test_filtering.py
@fege fege merged commit 5e62a40 into opendatahub-io:main Mar 16, 2026
10 checks passed
@fege fege deleted the spdx_tests branch March 16, 2026 15:29
@github-actions
Copy link
Copy Markdown

Status of building tag latest: success.
Status of pushing tag latest to image registry: success.

ssaleem-rh pushed a commit to ssaleem-rh/opendatahub-tests that referenced this pull request Mar 23, 2026
Signed-off-by: fege <fmosca@redhat.com>
Co-authored-by: Debarati Basu-Nag <dbasunag@redhat.com>
Signed-off-by: Shehan Saleem <ssaleem@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants