Moved advanced settings acceptance tests into Admin - #29395
Merged
Conversation
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run @tryghost/admin:test:acceptance |
✅ Succeeded | 5m 11s | View ↗ |
nx run-many -t test:unit -p @tryghost/admin-x-s... |
✅ Succeeded | 2m 20s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 1m 56s | View ↗ |
nx run ghost-monorepo:lint:boundaries |
✅ Succeeded | 20s | View ↗ |
nx run-many -t lint -p @tryghost/admin-x-settin... |
✅ Succeeded | 1m 5s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | 1s | View ↗ |
nx run @tryghost/admin-x-settings:test:acceptance |
✅ Succeeded | 6s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-16 16:51:46 UTC
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds Admin browser acceptance tests for Advanced settings and integrations, covering settings updates, danger-zone actions, filters, migration and export tools, Labs operations, history, integration CRUD, validation, uploads, API keys, plan limits, and ordering. Extends shared test utilities with typed integration and action fakes, removes corresponding Admin X suites, and replaces the Signup Embed scaffold with a placeholder. Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
9larsons
force-pushed
the
codex/settings-test-batch-7
branch
from
July 16, 2026 16:16
d3e5c0c to
5ded287
Compare
9larsons
marked this pull request as ready for review
July 16, 2026 16:17
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/admin/src/settings/advanced/integrations.acceptance.test.tsx (1)
144-145: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winKeep this interaction inside the browser locator API.
Unwrapping the element and calling native
click()bypasses locator actionability checks and retries used elsewhere in this suite.Proposed change
- const regenerateButton = modal.getByRole("button", {name: "Regenerate"}).element() as HTMLButtonElement; - regenerateButton.click(); + await modal.getByRole("button", {name: "Regenerate"}).click();🤖 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 `@apps/admin/src/settings/advanced/integrations.acceptance.test.tsx` around lines 144 - 145, Update the regeneration interaction in the acceptance test to invoke the locator’s click action directly, keeping it within the browser locator API. Remove the element unwrapping and native HTMLButtonElement.click() call while preserving the existing Regenerate button lookup.
🤖 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.
Nitpick comments:
In `@apps/admin/src/settings/advanced/integrations.acceptance.test.tsx`:
- Around line 144-145: Update the regeneration interaction in the acceptance
test to invoke the locator’s click action directly, keeping it within the
browser locator API. Remove the element unwrapping and native
HTMLButtonElement.click() call while preserving the existing Regenerate button
lookup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1a00a726-3643-48f2-878d-c86675e35bf1
📒 Files selected for processing (18)
apps/admin-x-settings/test/acceptance/advanced/code-injection.test.tsapps/admin-x-settings/test/acceptance/advanced/dangerzone.test.tsapps/admin-x-settings/test/acceptance/advanced/history.test.tsapps/admin-x-settings/test/acceptance/advanced/integrations/custom.test.tsapps/admin-x-settings/test/acceptance/advanced/integrations/first-promoter.test.tsapps/admin-x-settings/test/acceptance/advanced/integrations/integrations-list.test.tsapps/admin-x-settings/test/acceptance/advanced/integrations/pintura.test.tsapps/admin-x-settings/test/acceptance/advanced/integrations/slack.test.tsapps/admin-x-settings/test/acceptance/advanced/integrations/transistor.test.tsapps/admin-x-settings/test/acceptance/advanced/integrations/unsplash.test.tsapps/admin-x-settings/test/acceptance/advanced/integrations/zapier.test.tsapps/admin-x-settings/test/acceptance/advanced/labs.test.tsapps/admin-x-settings/test/acceptance/advanced/migration-tools.test.tsapps/admin-x-settings/test/acceptance/advanced/spam-filters.test.tsapps/admin/src/settings/advanced/advanced.acceptance.test.tsxapps/admin/src/settings/advanced/integrations.acceptance.test.tsxapps/admin/test-utils/acceptance/index.tsapps/admin/test-utils/acceptance/resources.ts
💤 Files with no reviewable changes (14)
- apps/admin-x-settings/test/acceptance/advanced/integrations/slack.test.ts
- apps/admin-x-settings/test/acceptance/advanced/migration-tools.test.ts
- apps/admin-x-settings/test/acceptance/advanced/integrations/integrations-list.test.ts
- apps/admin-x-settings/test/acceptance/advanced/integrations/unsplash.test.ts
- apps/admin-x-settings/test/acceptance/advanced/integrations/first-promoter.test.ts
- apps/admin-x-settings/test/acceptance/advanced/labs.test.ts
- apps/admin-x-settings/test/acceptance/advanced/spam-filters.test.ts
- apps/admin-x-settings/test/acceptance/advanced/dangerzone.test.ts
- apps/admin-x-settings/test/acceptance/advanced/code-injection.test.ts
- apps/admin-x-settings/test/acceptance/advanced/integrations/transistor.test.ts
- apps/admin-x-settings/test/acceptance/advanced/history.test.ts
- apps/admin-x-settings/test/acceptance/advanced/integrations/zapier.test.ts
- apps/admin-x-settings/test/acceptance/advanced/integrations/custom.test.ts
- apps/admin-x-settings/test/acceptance/advanced/integrations/pintura.test.ts
ref https://linear.app/ghost/issue/PLA-243 Consolidated the final test-bearing advanced settings suites into the Admin acceptance harness while preserving stronger E2E ownership.
9larsons
force-pushed
the
codex/settings-test-batch-7
branch
from
July 16, 2026 16:24
5ded287 to
7d02b28
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@apps/admin-x-settings/test/acceptance/membership/signup-embed.test.ts`:
- Around line 3-6: Update the “Settings acceptance placeholder” test to be
explicitly skipped or marked as fixme, and include a reference to the follow-up
teardown or implementation issue. Do not leave it as an always-passing test with
no assertions or browser actions.
In `@apps/admin/src/settings/advanced/advanced.acceptance.test.tsx`:
- Line 196: Update the usersApi request predicate in the expect.poll assertion
to parse each request URL and read the limit query parameter, matching the
established approach used for actionsApi. Compare the parsed parameter value
with the expected limit while preserving the existing polling behavior.
🪄 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: b4beef07-9867-49a8-bc9a-8ccc8dd1f7c6
📒 Files selected for processing (19)
apps/admin-x-settings/test/acceptance/advanced/code-injection.test.tsapps/admin-x-settings/test/acceptance/advanced/dangerzone.test.tsapps/admin-x-settings/test/acceptance/advanced/history.test.tsapps/admin-x-settings/test/acceptance/advanced/integrations/custom.test.tsapps/admin-x-settings/test/acceptance/advanced/integrations/first-promoter.test.tsapps/admin-x-settings/test/acceptance/advanced/integrations/integrations-list.test.tsapps/admin-x-settings/test/acceptance/advanced/integrations/pintura.test.tsapps/admin-x-settings/test/acceptance/advanced/integrations/slack.test.tsapps/admin-x-settings/test/acceptance/advanced/integrations/transistor.test.tsapps/admin-x-settings/test/acceptance/advanced/integrations/unsplash.test.tsapps/admin-x-settings/test/acceptance/advanced/integrations/zapier.test.tsapps/admin-x-settings/test/acceptance/advanced/labs.test.tsapps/admin-x-settings/test/acceptance/advanced/migration-tools.test.tsapps/admin-x-settings/test/acceptance/advanced/spam-filters.test.tsapps/admin-x-settings/test/acceptance/membership/signup-embed.test.tsapps/admin/src/settings/advanced/advanced.acceptance.test.tsxapps/admin/src/settings/advanced/integrations.acceptance.test.tsxapps/admin/test-utils/acceptance/index.tsapps/admin/test-utils/acceptance/resources.ts
💤 Files with no reviewable changes (14)
- apps/admin-x-settings/test/acceptance/advanced/integrations/zapier.test.ts
- apps/admin-x-settings/test/acceptance/advanced/labs.test.ts
- apps/admin-x-settings/test/acceptance/advanced/integrations/integrations-list.test.ts
- apps/admin-x-settings/test/acceptance/advanced/integrations/custom.test.ts
- apps/admin-x-settings/test/acceptance/advanced/spam-filters.test.ts
- apps/admin-x-settings/test/acceptance/advanced/history.test.ts
- apps/admin-x-settings/test/acceptance/advanced/code-injection.test.ts
- apps/admin-x-settings/test/acceptance/advanced/integrations/pintura.test.ts
- apps/admin-x-settings/test/acceptance/advanced/integrations/first-promoter.test.ts
- apps/admin-x-settings/test/acceptance/advanced/migration-tools.test.ts
- apps/admin-x-settings/test/acceptance/advanced/integrations/transistor.test.ts
- apps/admin-x-settings/test/acceptance/advanced/integrations/slack.test.ts
- apps/admin-x-settings/test/acceptance/advanced/integrations/unsplash.test.ts
- apps/admin-x-settings/test/acceptance/advanced/dangerzone.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/admin/test-utils/acceptance/index.ts
- apps/admin/test-utils/acceptance/resources.ts
- apps/admin/src/settings/advanced/integrations.acceptance.test.tsx
9larsons
enabled auto-merge (squash)
July 16, 2026 16:30
9larsons
disabled auto-merge
July 16, 2026 16:35
ref https://linear.app/tryghost/issue/PLA-243 - kept the placeholder discoverable without reporting a false pass - used locator interaction for API key regeneration
9larsons
enabled auto-merge (squash)
July 16, 2026 16:50
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
1 participant
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.
What changed
fakeActionsandfakeIntegrationsacceptance resourceshello worldPlaywright test so the existing Settings acceptance CI target does not fail withNo tests foundAfter this PR, only the temporary placeholder remains in the legacy Settings acceptance lane. The follow-up teardown slice can remove it together with the old Playwright target, config, dependency, and fixtures.
Validation
pnpm --filter admin test:acceptance(49 files, 322 tests)pnpm --filter admin-x-settings test:acceptance(1 placeholder test)pnpm --filter admin typecheckpnpm --filter admin-x-settings lintgit diff --checkFollow-ups
WebhookModalchanges an uncontrolled input to controlled while editing a custom webhook400after capturing the mutation to avoid navigation; the successful mutation is owned by the danger-zone E2E test, but this duplicate produces expected console noiseSwitchwarning; track down the owning settings flow separatelyref https://linear.app/ghost/issue/PLA-243