test: add MaaS subscription tests#1238
Merged
dbasunag merged 6 commits intoopendatahub-io:mainfrom Mar 17, 2026
Merged
Conversation
|
The following are automatically added/executed:
Available user actions:
Supported labels{'/hold', '/lgtm', '/wip', '/verified', '/cherry-pick', '/build-push-pr-image'} |
Signed-off-by: Swati Mukund Bagal <sbagal@redhat.com>
Contributor
📝 WalkthroughWalkthroughAdds three pytest artifacts: two new integration tests checking MaaS subscription authorization failure modes (ambiguous subscription without header; actor not in MaaSAuthPolicy), and two fixtures supporting multi-subscription and actor-scoped premium subscription provisioning. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
SB159
force-pushed
the
feature/maas-subscription-additional-test
branch
from
cf6fed5 to
13889f4
Compare
for more information, see https://pre-commit.ci
dbasunag
requested changes
Mar 17, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tests/model_serving/maas_billing/maas_subscription/test_multiple_subscriptions_no_header.py (2)
17-45: Redundant fixture declaration.
maas_subscription_tinyllama_freeis listed in bothusefixtures(line 25) and as a test parameter (line 42). Since you need the fixture value (used on line 76), remove it fromusefixturesand keep only the parameter.Proposed fix
`@pytest.mark.usefixtures`( "maas_unprivileged_model_namespace", "maas_subscription_controller_enabled_latest", "maas_gateway_api", "maas_api_gateway_reachable", "maas_inference_service_tinyllama_free", "maas_model_tinyllama_free", "maas_auth_policy_tinyllama_free", - "maas_subscription_tinyllama_free", )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/model_serving/maas_billing/maas_subscription/test_multiple_subscriptions_no_header.py` around lines 17 - 45, The maas_subscription_tinyllama_free fixture is redundantly declared both in the class-level pytest.mark.usefixtures and as a test parameter in TestMultipleSubscriptionsNoHeader; remove maas_subscription_tinyllama_free from the usefixtures decorator (leave it only as the test function parameter in test_multiple_matching_subscriptions_no_header_gets_403) so the test receives the fixture value correctly when referenced later in the test.
55-55: Dead code.This line assigns to
_(throwaway), but you actually usemaas_subscription_tinyllama_free.nameon line 76. Remove the no-op.Proposed fix
- _ = maas_subscription_tinyllama_free - with create_maas_subscription(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/model_serving/maas_billing/maas_subscription/test_multiple_subscriptions_no_header.py` at line 55, Remove the dead no-op assignment to `_`—delete the line that does "_ = maas_subscription_tinyllama_free" so the fixture/variable maas_subscription_tinyllama_free remains available for later use (e.g., accessing maas_subscription_tinyllama_free.name in the test). Ensure no other unused throwaway assignments remain for that fixture.tests/model_serving/maas_billing/maas_subscription/test_subscription_without_auth_policy.py (1)
19-45: Redundant fixture declaration.Same issue as the other test file:
maas_subscription_tinyllama_premiumis in bothusefixtures(line 27) and as a test parameter (line 44). Remove fromusefixtures.Proposed fix
`@pytest.mark.usefixtures`( "maas_unprivileged_model_namespace", "maas_subscription_controller_enabled_latest", "maas_gateway_api", "maas_api_gateway_reachable", "maas_inference_service_tinyllama_premium", "maas_model_tinyllama_premium", "maas_auth_policy_tinyllama_premium", - "maas_subscription_tinyllama_premium", )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/model_serving/maas_billing/maas_subscription/test_subscription_without_auth_policy.py` around lines 19 - 45, The test declares the fixture maas_subscription_tinyllama_premium twice (in the class-level pytest.mark.usefixtures list and again as a test function parameter) causing redundancy; edit the class TestSubscriptionWithoutAuthPolicy to remove maas_subscription_tinyllama_premium from the usefixtures(...) list so the fixture is provided only via the test method parameter (leave the test signature and other fixtures intact).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@tests/model_serving/maas_billing/maas_subscription/test_multiple_subscriptions_no_header.py`:
- Around line 17-45: The maas_subscription_tinyllama_free fixture is redundantly
declared both in the class-level pytest.mark.usefixtures and as a test parameter
in TestMultipleSubscriptionsNoHeader; remove maas_subscription_tinyllama_free
from the usefixtures decorator (leave it only as the test function parameter in
test_multiple_matching_subscriptions_no_header_gets_403) so the test receives
the fixture value correctly when referenced later in the test.
- Line 55: Remove the dead no-op assignment to `_`—delete the line that does "_
= maas_subscription_tinyllama_free" so the fixture/variable
maas_subscription_tinyllama_free remains available for later use (e.g.,
accessing maas_subscription_tinyllama_free.name in the test). Ensure no other
unused throwaway assignments remain for that fixture.
In
`@tests/model_serving/maas_billing/maas_subscription/test_subscription_without_auth_policy.py`:
- Around line 19-45: The test declares the fixture
maas_subscription_tinyllama_premium twice (in the class-level
pytest.mark.usefixtures list and again as a test function parameter) causing
redundancy; edit the class TestSubscriptionWithoutAuthPolicy to remove
maas_subscription_tinyllama_premium from the usefixtures(...) list so the
fixture is provided only via the test method parameter (leave the test signature
and other fixtures intact).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: f46ef53a-96e9-4bc5-8be1-3f16d50d27ac
📒 Files selected for processing (2)
tests/model_serving/maas_billing/maas_subscription/test_multiple_subscriptions_no_header.pytests/model_serving/maas_billing/maas_subscription/test_subscription_without_auth_policy.py
Signed-off-by: Swati Mukund Bagal <sbagal@redhat.com>
Signed-off-by: Swati Mukund Bagal <sbagal@redhat.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/model_serving/maas_billing/maas_subscription/conftest.py (1)
573-616: Generate unique subscription names here, and keep the ambiguity case symmetric.Line 576 and Line 607 hard-code function-scoped cluster resource names, which is brittle on reruns, parallel workers, or delayed cleanup. Line 582 also sets
priority=5even though the baseline free subscription in this file usespriority=0, so the “multiple matching subscriptions” setup is no longer clearly modeling two equally eligible subscriptions. Usegenerate_random_name()for both names and match the baseline priority insecond_free_subscription.Suggested change
with create_maas_subscription( admin_client=admin_client, subscription_namespace=maas_subscription_tinyllama_free.namespace, - subscription_name="e2e-second-free-subscription", + subscription_name=f"e2e-second-free-subscription-{generate_random_name()}", owner_group_name=maas_free_group, model_name=maas_model_tinyllama_free.name, model_namespace=maas_model_tinyllama_free.namespace, tokens_per_minute=500, window="1m", - priority=5, + priority=0, teardown=True, wait_for_resource=True, ) as second_subscription: @@ with create_maas_subscription( admin_client=admin_client, subscription_namespace=maas_subscription_tinyllama_premium.namespace, - subscription_name="e2e-free-actor-premium-sub", + subscription_name=f"e2e-free-actor-premium-sub-{generate_random_name()}", owner_group_name="system:authenticated", model_name=maas_model_tinyllama_premium.name, model_namespace=maas_model_tinyllama_premium.namespace,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/model_serving/maas_billing/maas_subscription/conftest.py` around lines 573 - 616, Replace the hard-coded subscription names and mismatched priority by generating unique names and using the same priority as the baseline free subscription: call generate_random_name() when constructing the subscription_name for both the second free subscription (created in the with create_maas_subscription(...) as second_subscription block) and the free_actor_premium_subscription (the with create_maas_subscription(...) as sub_for_free_actor block), and change priority=5 in the second_free_subscription creation to priority=0 to match the baseline; keep all other arguments and teardown/wait_for_resource behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/model_serving/maas_billing/maas_subscription/conftest.py`:
- Around line 573-616: Replace the hard-coded subscription names and mismatched
priority by generating unique names and using the same priority as the baseline
free subscription: call generate_random_name() when constructing the
subscription_name for both the second free subscription (created in the with
create_maas_subscription(...) as second_subscription block) and the
free_actor_premium_subscription (the with create_maas_subscription(...) as
sub_for_free_actor block), and change priority=5 in the second_free_subscription
creation to priority=0 to match the baseline; keep all other arguments and
teardown/wait_for_resource behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 3e16d4e3-729f-43ef-81f7-ab41e021d8ad
📒 Files selected for processing (3)
tests/model_serving/maas_billing/maas_subscription/conftest.pytests/model_serving/maas_billing/maas_subscription/test_multiple_subscriptions_no_header.pytests/model_serving/maas_billing/maas_subscription/test_subscription_without_auth_policy.py
dbasunag
approved these changes
Mar 17, 2026
|
Status of building tag latest: success. |
ssaleem-rh
pushed a commit
to ssaleem-rh/opendatahub-tests
that referenced
this pull request
Mar 23, 2026
* test: add MaaS subscription tests Signed-off-by: Swati Mukund Bagal <sbagal@redhat.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * address review comments Signed-off-by: Swati Mukund Bagal <sbagal@redhat.com> * address review comments Signed-off-by: Swati Mukund Bagal <sbagal@redhat.com> --------- Signed-off-by: Swati Mukund Bagal <sbagal@redhat.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Debarati Basu-Nag <dbasunag@redhat.com> Signed-off-by: Shehan Saleem <ssaleem@redhat.com>
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
4 participants
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.
Pull Request
Summary
Some additional MaaS subscription tests.
Related Issues
How it has been tested
Additional Requirements
Summary by CodeRabbit