Skip to content

feat: Add Playwright option to bonfire IQE CJI deployment command#569

Merged
bsquizz merged 1 commit into
RedHatInsights:masterfrom
mmusil:add-iqe-cji-playwright-support
May 14, 2026
Merged

feat: Add Playwright option to bonfire IQE CJI deployment command#569
bsquizz merged 1 commit into
RedHatInsights:masterfrom
mmusil:add-iqe-cji-playwright-support

Conversation

@mmusil
Copy link
Copy Markdown
Contributor

@mmusil mmusil commented May 13, 2026

  • Add --playwright CLI flag to deploy-iqe-cji and process-iqe-cji commands
  • Update processor to handle playwright parameter and pass to templates
  • Add deploy_playwright parameter to render_cji() library function
  • Update CJI template to include playwright.deploy configuration
  • Add comprehensive tests for Playwright deployment modes

Supports three deployment modes:

  • Playwright only: bonfire deploy-iqe-cji app --playwright
  • Selenium only: bonfire deploy-iqe-cji app --selenium
  • Both: bonfire deploy-iqe-cji app --selenium --playwright

Tests added:

  • test_deploy_playwright: validates Playwright-only deployment
  • test_deploy_both_selenium_and_playwright: validates concurrent deployment
  • Updated test_defaults to verify both are disabled by default

Tested end-to-end against minikube with Clowder master (PR #1772). All deployment modes work correctly with proper container creation, volume sharing, and resource allocation.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

Warning

Rate limit exceeded

@mmusil has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 59 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 71d7ec37-c7d6-474f-ad47-553fe66813bc

📥 Commits

Reviewing files that changed from the base of the PR and between e114532 and e502905.

📒 Files selected for processing (5)
  • bonfire/bonfire.py
  • bonfire/processor.py
  • bonfire_lib/core_resources.py
  • bonfire_lib/templates/clowdjobinvocation.yaml.j2
  • tests/test_bonfire_lib/test_core_resources.py

Walkthrough

This PR adds a new --playwright CLI flag to bonfire's IQE ClowdJobInvocation processing and deployment commands. The flag is threaded through the CLI layer, processing layer, and template rendering to enable Playwright container deployment alongside the existing Selenium option.

Changes

Playwright Deployment Flag Integration

Layer / File(s) Summary
CLI flag and command injection
bonfire/bonfire.py
Added --playwright option to _iqe_cji_process_options, injected as a parameter into both _cmd_process_iqe_cji and _cmd_deploy_iqe_cji, and passed through to process_iqe_cji calls.
Processing and template parameter threading
bonfire/processor.py, bonfire_lib/core_resources.py
process_iqe_cji accepts new playwright parameter (default False) and sets DEPLOY_PLAYWRIGHT in template params; render_cji accepts deploy_playwright parameter and passes it to Jinja2 template rendering.
Template Playwright configuration
bonfire_lib/templates/clowdjobinvocation.yaml.j2
Added spec.testing.iqe.ui.playwright.deploy section templated from deploy_playwright with default false, mirroring existing Selenium UI configuration.
Test coverage
tests/test_bonfire_lib/test_core_resources.py
Updated default test to assert Playwright deploy flag defaults to false; added test cases for deploy_playwright=True only and combined deploy_selenium=True and deploy_playwright=True.

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: Add Playwright option to bonfire IQE CJI deployment command' accurately captures the main change: adding Playwright deployment support to the bonfire CLI commands.
Description check ✅ Passed The description is directly related to the changeset, covering the CLI flag addition, processor updates, template changes, and test additions with concrete deployment mode examples.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

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

Copy link
Copy Markdown

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/test_bonfire_lib/test_core_resources.py (1)

150-159: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Move IBUTSU override assertions out of the combined deploy test.

Line 156 assertions require custom IBUTSU inputs, but this test uses defaults, so it will fail for the wrong reason.

Suggested fix
@@
     def test_custom_values(self):
@@
         assert env_dict["IQE_PARALLEL_ENABLED"] == "false"
         assert env_dict["IQE_PARALLEL_WORKER_COUNT"] == "4"
+        value_from_dict = {e["name"]: e["valueFrom"] for e in iqe["env"] if "valueFrom" in e}
+        assert value_from_dict["IBUTSU_MODE"]["configMapKeyRef"]["name"] == "my-ibutsu-cm"
+        assert value_from_dict["IBUTSU_PROJECT"]["configMapKeyRef"]["name"] == "my-ibutsu-cm"
+        assert value_from_dict["IBUTSU_TOKEN"]["secretKeyRef"]["name"] == "my-ibutsu-secret"
@@
     def test_deploy_both_selenium_and_playwright(self):
         result = render_cji("test-cji", "my-app", deploy_selenium=True, deploy_playwright=True)
         iqe = result["spec"]["testing"]["iqe"]
         assert iqe["ui"]["selenium"]["deploy"] is True
         assert iqe["ui"]["playwright"]["deploy"] is True
-
-        value_from_dict = {e["name"]: e["valueFrom"] for e in iqe["env"] if "valueFrom" in e}
-        assert value_from_dict["IBUTSU_MODE"]["configMapKeyRef"]["name"] == "my-ibutsu-cm"
-        assert value_from_dict["IBUTSU_PROJECT"]["configMapKeyRef"]["name"] == "my-ibutsu-cm"
-        assert value_from_dict["IBUTSU_TOKEN"]["secretKeyRef"]["name"] == "my-ibutsu-secret"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_bonfire_lib/test_core_resources.py` around lines 150 - 159, The
assertions checking IBUTSU configMap/secret names in
test_deploy_both_selenium_and_playwright are using custom IBUTSU inputs that
this test does not provide; remove the three assertions that reference
IBUTSU_MODE, IBUTSU_PROJECT, and IBUTSU_TOKEN from
test_deploy_both_selenium_and_playwright (the test that calls render_cji with
deploy_selenium=True and deploy_playwright=True) and either add a separate test
that calls render_cji with the custom IBUTSU inputs to assert those valueFrom
entries, or update/rename an existing IBUTSU-specific test to include those
assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@tests/test_bonfire_lib/test_core_resources.py`:
- Around line 150-159: The assertions checking IBUTSU configMap/secret names in
test_deploy_both_selenium_and_playwright are using custom IBUTSU inputs that
this test does not provide; remove the three assertions that reference
IBUTSU_MODE, IBUTSU_PROJECT, and IBUTSU_TOKEN from
test_deploy_both_selenium_and_playwright (the test that calls render_cji with
deploy_selenium=True and deploy_playwright=True) and either add a separate test
that calls render_cji with the custom IBUTSU inputs to assert those valueFrom
entries, or update/rename an existing IBUTSU-specific test to include those
assertions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 15fc02ba-bd60-44a7-a6ec-7c702bd3023a

📥 Commits

Reviewing files that changed from the base of the PR and between 421d6e1 and e114532.

📒 Files selected for processing (5)
  • bonfire/bonfire.py
  • bonfire/processor.py
  • bonfire_lib/core_resources.py
  • bonfire_lib/templates/clowdjobinvocation.yaml.j2
  • tests/test_bonfire_lib/test_core_resources.py

- Add --playwright CLI flag to deploy-iqe-cji and process-iqe-cji commands
- Update processor to handle playwright parameter and pass to templates
- Add deploy_playwright parameter to render_cji() library function
- Update CJI template to include playwright.deploy configuration
- Add comprehensive tests for Playwright deployment modes

Supports three deployment modes:
- Playwright only: bonfire deploy-iqe-cji app --playwright
- Selenium only: bonfire deploy-iqe-cji app --selenium
- Both: bonfire deploy-iqe-cji app --selenium --playwright

Tests added:
- test_deploy_playwright: validates Playwright-only deployment
- test_deploy_both_selenium_and_playwright: validates concurrent deployment
- Updated test_defaults to verify both are disabled by default

Tested end-to-end against minikube with Clowder master (PR #1772).
All deployment modes work correctly with proper container creation,
volume sharing, and resource allocation.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@bsquizz bsquizz merged commit 13e2356 into RedHatInsights:master May 14, 2026
12 checks passed
@mmusil mmusil deleted the add-iqe-cji-playwright-support branch May 15, 2026 06:52
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.

2 participants