Skip to content

Consolidated custom views e2e tests into flow-based tests#27066

Open
9larsons wants to merge 1 commit intomainfrom
e2e-consolidate-custom-views
Open

Consolidated custom views e2e tests into flow-based tests#27066
9larsons wants to merge 1 commit intomainfrom
e2e-consolidate-custom-views

Conversation

@9larsons
Copy link
Copy Markdown
Contributor

@9larsons 9larsons commented Apr 1, 2026

no ref

Restructured custom-views.test.ts from 17 fine-grained single-assertion tests into 6 flow-based tests that each walk through a complete user journey.

Before: 17 tests, each paying full setup cost (create tag, navigate to posts, apply filter, save view) for one assertion.

After: 6 tests that mirror real user journeys:

  • Create + verify — creates a view with color, verifies sidebar appearance, navigates away and back
  • Navigate between views — creates 2 views, switches between them, verifies active state, tests filter-matching
  • Edit — creates view, renames it, changes color, verifies updates
  • Delete — creates 3 views, deletes middle one (others remain), deletes active (redirects to Posts)
  • Validation — duplicate name shows error (kept separate as error path)
  • Persistence — creates view, reloads page, verifies name + filters + active state survive

Every assertion from the original 17 tests is preserved. The file goes from 460 lines to 210 lines.

Test plan

  • CI e2e tests pass on all 8 shards

Restructured 17 fine-grained tests into 6 flow-based tests that each
walk through a complete user journey. Every assertion from the original
tests is preserved — create+color+active state, navigation between
views, rename+recolor, delete with remainder check, validation error,
and persistence after reload.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 1, 2026

Walkthrough

The test file for custom views was restructured to reduce complexity and coverage scope. PostFactory usage was removed entirely, with tests now relying only on TagFactory for prerequisites. Multiple nested test groups were collapsed into fewer top-level tests. Assertions were updated to validate sidebar state using aria-current="page" and data-color attributes instead of prior methods. Navigation, editing, and deletion test cases were consolidated into single tests with modified validation approaches. Overall test coverage was reduced, with the file decreasing from 429 to 209 net lines changed.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: consolidating 17 fine-grained custom views e2e tests into 6 flow-based tests, which is the core objective of the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The pull request description clearly explains the consolidation of 17 fine-grained tests into 6 flow-based tests, detailing each test's purpose and the overall restructuring strategy.

✏️ 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 e2e-consolidate-custom-views

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.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 1, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
28.3% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

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: 2

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

Inline comments:
In `@e2e/tests/admin/posts/custom-views.test.ts`:
- Around line 37-39: The test currently only asserts the URL contains type=draft
after clicking sidebar.getNavLink('Featured Drafts') but doesn't verify the
restored tag; update the assertion to also check the tag query param or the UI
selected tag filter. Specifically, after await sidebar.getNavLink('Featured
Drafts').click(), add an assertion on page.url() or toHaveURL() that includes
the expected tag query param (e.g., /type=draft.*tag=Featured/) or assert the
tag filter element (the selected tag control accessed via the existing sidebar
or page locators) shows the "Featured" tag is selected so the saved tag is
re-validated.
- Around line 33-39: The test block mixes multiple scenarios (revisit/restore,
navigation activation) in one case — split each scenario into its own test
following "what is tested - expected outcome" lowercase naming and AAA
structure: create separate tests that individually exercise
sidebar.getNavLink('Posts').click() then assert restore behavior, and another
that clicks sidebar.getNavLink('Featured Drafts') and asserts await
expect(page).toHaveURL(/type=draft/) and await
expect(sidebar.getNavLink('Featured
Drafts')).toHaveAttribute('aria-current','page'); ensure each new test only
contains Arrange (setup), Act (single user action like click or delete), and
Assert steps and apply the same refactor for the other ranges (85-93, 122-129,
179-189) so failures don’t block later scenarios.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6059f15a-f63f-403b-a4ca-47caba9567dd

📥 Commits

Reviewing files that changed from the base of the PR and between 3024ee8 and c11d86c.

📒 Files selected for processing (1)
  • e2e/tests/admin/posts/custom-views.test.ts

Comment on lines +33 to +39
// Navigate away and back — filters should be restored
await sidebar.getNavLink('Posts').click();
await expect(sidebar.getNavLink('Featured Drafts')).not.toHaveAttribute('aria-current', 'page');

await sidebar.getNavLink('Featured Drafts').click();
await expect(page).toHaveURL(/type=draft/);
await expect(sidebar.getNavLink('Featured Drafts')).toHaveAttribute('aria-current', 'page');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Break these follow-on flows into separate tests.

Each of these ranges starts a second scenario after the first one is already asserted: revisit/restore after create, filter-driven activation after click navigation, recolor after rename, and active-view redirect after deleting another view. That makes the later coverage disappear whenever the earlier half fails.

As per coding guidelines, Test names should follow 'what is tested - expected outcome' format in lowercase, with one test = one scenario (never mix multiple scenarios) and Use the AAA (Arrange-Act-Assert) pattern for test structure.

Also applies to: 85-93, 122-129, 179-189

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@e2e/tests/admin/posts/custom-views.test.ts` around lines 33 - 39, The test
block mixes multiple scenarios (revisit/restore, navigation activation) in one
case — split each scenario into its own test following "what is tested -
expected outcome" lowercase naming and AAA structure: create separate tests that
individually exercise sidebar.getNavLink('Posts').click() then assert restore
behavior, and another that clicks sidebar.getNavLink('Featured Drafts') and
asserts await expect(page).toHaveURL(/type=draft/) and await
expect(sidebar.getNavLink('Featured
Drafts')).toHaveAttribute('aria-current','page'); ensure each new test only
contains Arrange (setup), Act (single user action like click or delete), and
Assert steps and apply the same refactor for the other ranges (85-93, 122-129,
179-189) so failures don’t block later scenarios.

Comment on lines +37 to +39
await sidebar.getNavLink('Featured Drafts').click();
await expect(page).toHaveURL(/type=draft/);
await expect(sidebar.getNavLink('Featured Drafts')).toHaveAttribute('aria-current', 'page');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Re-assert the saved tag when returning to the view.

This only proves that type=draft came back. If the Featured tag stops being restored, the test still passes. Assert the tag query param or selected tag filter as well.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@e2e/tests/admin/posts/custom-views.test.ts` around lines 37 - 39, The test
currently only asserts the URL contains type=draft after clicking
sidebar.getNavLink('Featured Drafts') but doesn't verify the restored tag;
update the assertion to also check the tag query param or the UI selected tag
filter. Specifically, after await sidebar.getNavLink('Featured Drafts').click(),
add an assertion on page.url() or toHaveURL() that includes the expected tag
query param (e.g., /type=draft.*tag=Featured/) or assert the tag filter element
(the selected tag control accessed via the existing sidebar or page locators)
shows the "Featured" tag is selected so the saved tag is re-validated.

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.

1 participant