Let's fix PyTest suite for cakephp ex repository

[phracek]

Summary by CodeRabbit

• Chores

  • Updated the test workflow labeling to use an OS-based naming scheme and align PyTest status names with the current test matrix.

• Tests

  • Improved OpenShift testing by gating execution to supported OS/version combinations.
  • Adjusted CakePHP template deployment tests to run in a shared cluster context, simplified template selection, and added a new local-template scenario.
  • Standardized expected output checks and increased deployment verification timeout.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4bb68d9a-4677-408d-a529-cffb57474073

📥 Commits

Reviewing files that changed from the base of the PR and between 5b47a34 and 2f297f1.

📒 Files selected for processing (4)

• .github/workflows/openshift-tests.yml
• tests/__init__.py
• tests/constants.py
• tests/test_cakephp.py

---

📝 Walkthrough

Walkthrough

The PR refactors the CakePHP test harness to support multi-OS testing with version gating. It introduces test constants defining allowed PHP versions per RHEL release (TAGS and is_test_allowed), configures both template tests to skip unsupported OS-version combinations, updates OpenShiftAPI to use a shared cluster, adds a new local template deployment test, refactors the remote test to use PR-based source references instead of version-dependent branch selection, and updates the workflow PR status label to include the OS matrix variable.

Changes

CakePHP Test Harness: OS-Version Gating and Template Refactor

Layer / File(s)	Summary
Test constants: TAGS and OS-version allowlists tests/constants.py	Introduces TAGS mapping for RHEL UBI suffixes and is_test_allowed(os, version) that gates tests by predefined OS-version combinations: RHEL8 supports PHP 7.4 and 8.2; RHEL9 supports 8.0, 8.2, 8.3; RHEL10 supports 8.3, 8.4.
Environment setup and shared cluster fixture tests/test_cakephp.py (imports, module-level vars, fixture)	Imports TAGS and is_test_allowed from constants; reads OS, PR_NUMBER, and TAG from environment; removes fallback default for VERSION; configures OpenShiftAPI with shared_cluster=True.
Local template deployment test tests/test_cakephp.py (test_local_template_inside_cluster)	New test that gates by OS-version allowlist, deploys from local cakephp.json template, passes SOURCE_REPOSITORY_REF=refs/pull/{PR_NUMBER}/head and PHP_VERSION={VERSION}{TAG} as openshift_args, and verifies constant "Welcome to CakePHP" output with 600-second timeout.
Remote template deployment test refactor tests/test_cakephp.py (test_remote_template_inside_cluster)	Refactored test that gates by OS-version allowlist, removes version-based branch selection, retrieves fixed remote template JSON, passes PR-based source reference and versioned PHP tag in openshift_args, and validates constant expected output with 600-second timeout.
Workflow PR status label update .github/workflows/openshift-tests.yml	Changes pull_request_status_name from fixed RHEL9-OpenShift-4 - imagestream test to dynamic ${{ matrix.os }}-OpenShift-4 - PyTest scheme.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 A carrot for testing with care,
OS-version gating now everywhere!
Local and remote templates align,
PR-based refs and timeouts so fine—
Shared clusters hopping, the tests shine! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% 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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Let's fix PyTest suite for cakephp ex repository' directly aligns with the changeset, which updates the PyTest suite configuration and logic in test_cakephp.py and related workflow files.
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 docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch add_pytest_suite

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[phracek]

[test]

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

.github/workflows/openshift-tests.yml (1)

13-13: ⚡ Quick win

Consider whether PHP 8.1-ubi8 should be included in the test matrix.

The test matrix includes PHP 7.4, 8.2, 8.3, and 8.4, but skips 8.1. Since the default fallback version in tests/test_cakephp.py is "8.1-ubi8" and it represents the minimum version for CakePHP 5.X testing (based on the branching logic), consider adding it to ensure comprehensive coverage of the CakePHP 5 compatibility range.

🤖 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 @.github/workflows/openshift-tests.yml at line 13, Add "8.1-ubi8" to the
version matrix array in the openshift-tests.yml file. This version is currently
missing from the test matrix but is defined as the default fallback version in
tests/test_cakephp.py and represents the minimum version for CakePHP 5.X
testing. Include it in the appropriate position within the version array to
ensure comprehensive test coverage of the supported PHP versions.

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

Inline comments:
In `@tests/test_cakephp.py`:
- Around line 26-30: The version check in the conditional statement for
branch_to_test and expected_output currently only handles PHP 7.4 and 8.0, but
this causes PHP 8.1 to default to CakePHP 5 which requires PHP 8.2 minimum. Add
"8.1" to the version check condition alongside "7.4" and "8.0" so that PHP 8.1
environments are explicitly routed to use CakePHP 4.X, which supports PHP 8.1.
- Line 16: The OpenShiftAPI constructor call in the test setup is passing an
undocumented shared_cluster=True parameter that is not part of the library's
documented API and will cause issues at runtime. Remove the shared_cluster=True
parameter from the OpenShiftAPI instantiation while keeping the pod_name_prefix
parameter intact, matching the pattern used in similar test classes like
TestCakePHPAppMySQLExTemplate.

---

Nitpick comments:
In @.github/workflows/openshift-tests.yml:
- Line 13: Add "8.1-ubi8" to the version matrix array in the openshift-tests.yml
file. This version is currently missing from the test matrix but is defined as
the default fallback version in tests/test_cakephp.py and represents the minimum
version for CakePHP 5.X testing. Include it in the appropriate position within
the version array to ensure comprehensive test coverage of the supported PHP
versions.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d9111cb9-e334-4dc2-a41e-1feab19a2a5d

📥 Commits

Reviewing files that changed from the base of the PR and between 539f058 and 5b47a34.

📒 Files selected for processing (2)

• .github/workflows/openshift-tests.yml
• tests/test_cakephp.py

[phracek]

[test]