Skip to content

MPT-19967 refactor fixtures and remove redundant skip markers#298

Merged
jentyk merged 1 commit intomainfrom
fix/MPT-19967
Apr 13, 2026
Merged

MPT-19967 refactor fixtures and remove redundant skip markers#298
jentyk merged 1 commit intomainfrom
fix/MPT-19967

Conversation

@jentyk
Copy link
Copy Markdown
Member

@jentyk jentyk commented Apr 13, 2026

Closes MPT-19967

  • Refactor: moved Helpdesk parameter fixtures to a centralized parent conftest (tests/e2e/helpdesk/conftest.py)
  • Added centralized fixtures: parameter_data(short_uuid), invalid_parameter_id() (session), created_parameter(...), and async_created_parameter(...)
  • Updated parameter-group fixtures to use created_parameter/async_created_parameter and simplified creation payloads
  • Removed redundant fixtures file: tests/e2e/helpdesk/parameters/conftest.py
  • Tests unskipped: removed pytest.mark.skip from six e2e test modules so they now run under pytest.mark.flaky:
    • tests/e2e/helpdesk/parameter_groups/parameters/test_async_parameters.py
    • tests/e2e/helpdesk/parameter_groups/parameters/test_sync_parameters.py
    • tests/e2e/helpdesk/parameter_groups/test_async_parameter_groups.py
    • tests/e2e/helpdesk/parameter_groups/test_sync_parameter_groups.py
    • tests/e2e/helpdesk/parameters/test_async_parameters.py
    • tests/e2e/helpdesk/parameters/test_sync_parameters.py

@jentyk jentyk requested a review from a team as a code owner April 13, 2026 14:47
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Pro

Run ID: 2144ed17-c332-4377-8773-6239485d7fcb

📥 Commits

Reviewing files that changed from the base of the PR and between d36c667 and d4a1f81.

📒 Files selected for processing (9)
  • tests/e2e/helpdesk/conftest.py
  • tests/e2e/helpdesk/parameter_groups/parameters/conftest.py
  • tests/e2e/helpdesk/parameter_groups/parameters/test_async_parameters.py
  • tests/e2e/helpdesk/parameter_groups/parameters/test_sync_parameters.py
  • tests/e2e/helpdesk/parameter_groups/test_async_parameter_groups.py
  • tests/e2e/helpdesk/parameter_groups/test_sync_parameter_groups.py
  • tests/e2e/helpdesk/parameters/conftest.py
  • tests/e2e/helpdesk/parameters/test_async_parameters.py
  • tests/e2e/helpdesk/parameters/test_sync_parameters.py
💤 Files with no reviewable changes (1)
  • tests/e2e/helpdesk/parameters/conftest.py
✅ Files skipped from review due to trivial changes (5)
  • tests/e2e/helpdesk/parameter_groups/test_sync_parameter_groups.py
  • tests/e2e/helpdesk/parameter_groups/parameters/test_async_parameters.py
  • tests/e2e/helpdesk/parameter_groups/parameters/test_sync_parameters.py
  • tests/e2e/helpdesk/conftest.py
  • tests/e2e/helpdesk/parameter_groups/test_async_parameter_groups.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • tests/e2e/helpdesk/parameters/test_async_parameters.py
  • tests/e2e/helpdesk/parameters/test_sync_parameters.py

📝 Walkthrough

Walkthrough

This change moves helpdesk parameter fixtures into the parent helpdesk conftest, updates parameter-group-parameter fixtures to consume the relocated created-parameter fixtures, and removes module-level skip markers so parameter-related e2e tests run under the flaky marker.

Changes

Cohort / File(s) Summary
Helpdesk root conftest
tests/e2e/helpdesk/conftest.py
Added fixtures: parameter_data(short_uuid), invalid_parameter_id() (session), created_parameter(mpt_ops, parameter_data), async_created_parameter(async_mpt_ops, parameter_data) for shared parameter creation.
Removed nested parameters conftest
tests/e2e/helpdesk/parameters/conftest.py
Deleted local fixtures that were moved to the parent conftest: parameter_data, invalid_parameter_id, created_parameter, async_created_parameter.
Parameter-group parameter fixtures
tests/e2e/helpdesk/parameter_groups/parameters/conftest.py
Switched created_parameter_group_parameter and async_created_parameter_group_parameter to accept created_parameter / async_created_parameter and send payloads using {"id": <param.id>, "displayOrder": 1} instead of the prior parameter-group-parameter data fixture.
Re-enabled tests (removed skip markers)
tests/e2e/helpdesk/parameters/test_sync_parameters.py, tests/e2e/helpdesk/parameters/test_async_parameters.py, tests/e2e/helpdesk/parameter_groups/test_sync_parameter_groups.py, tests/e2e/helpdesk/parameter_groups/test_async_parameter_groups.py, tests/e2e/helpdesk/parameter_groups/parameters/test_sync_parameters.py, tests/e2e/helpdesk/parameter_groups/parameters/test_async_parameters.py
Removed pytest.mark.skip(reason="Unskip after MPT-19967 fixed") from module-level pytestmark in multiple test modules, leaving pytest.mark.flaky only, so those tests are no longer globally skipped.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Jira Issue Key In Title ✅ Passed The PR title contains the Jira issue key MPT-19967 in the correct format at the beginning, matching the required pattern MPT-XXXX.
Test Coverage Required ✅ Passed PR modifies only test files within tests/ folder with no modifications to code outside tests/. Changes involve fixture refactoring and skip marker removal.
Single Commit Required ✅ Passed The PR contains exactly one commit (hash: d4a1f81, message: 'test(helpdesk): refactor fixtures and remove redundant skip markers'). Multiple independent git commands confirm the total commit count is 1.

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


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.

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/e2e/helpdesk/conftest.py`:
- Line 149: The payload contains a mismatched key "DisplayOrder" which should be
"displayOrder" to match the rest of the fixtures; locate the dict/fixture in
conftest.py where "DisplayOrder": None is set and rename the key to
"displayOrder": None, and scan the file for other occurrences of "DisplayOrder"
to replace so all payloads use the same camelCase key.
🪄 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: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 5fe8e44f-fa1e-4dab-ba97-16748cf49dec

📥 Commits

Reviewing files that changed from the base of the PR and between b14414e and d36c667.

📒 Files selected for processing (9)
  • tests/e2e/helpdesk/conftest.py
  • tests/e2e/helpdesk/parameter_groups/parameters/conftest.py
  • tests/e2e/helpdesk/parameter_groups/parameters/test_async_parameters.py
  • tests/e2e/helpdesk/parameter_groups/parameters/test_sync_parameters.py
  • tests/e2e/helpdesk/parameter_groups/test_async_parameter_groups.py
  • tests/e2e/helpdesk/parameter_groups/test_sync_parameter_groups.py
  • tests/e2e/helpdesk/parameters/conftest.py
  • tests/e2e/helpdesk/parameters/test_async_parameters.py
  • tests/e2e/helpdesk/parameters/test_sync_parameters.py
💤 Files with no reviewable changes (1)
  • tests/e2e/helpdesk/parameters/conftest.py

Comment thread tests/e2e/helpdesk/conftest.py Outdated
@sonarqubecloud
Copy link
Copy Markdown

@jentyk jentyk merged commit 11651c9 into main Apr 13, 2026
4 checks passed
@jentyk jentyk deleted the fix/MPT-19967 branch April 13, 2026 15:04
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