Skip to content

Add Site Management UI integration tests#3950

Open
gcsecsey wants to merge 6 commits into
trunkfrom
gcsecsey/migrate-e2e-tests-to-cli
Open

Add Site Management UI integration tests#3950
gcsecsey wants to merge 6 commits into
trunkfrom
gcsecsey/migrate-e2e-tests-to-cli

Conversation

@gcsecsey

@gcsecsey gcsecsey commented Jun 24, 2026

Copy link
Copy Markdown
Member

Related issues

How AI was used in this PR

Claude audited the existing renderer tests to find coverage gaps, and wrote the tests.

Proposed Changes

The Site Management e2e tests drive every action through the UI, which is slow, flaky, and couples coverage to the current renderer. These tests take the UI part of the CLI/UI split we agreed on in the huddle: they mount the real components and SiteDetailsProvider, mock only the IPC bridge, and assert the commands each user action generates. The CLI integration tests in #3947 cover the parameter variations and on-disk effects.

  • Covers all nine Site Management smoke-sheet rows, plus custom domain and HTTPS.
  • Adds coverage for site duplication, since there is no studio site duplicate command for the CLI suite to test.
  • The delete cases now run on every platform (the old e2e versions were skipped on Windows).
  • These supersede the hook-boundary assertions in the sibling unit tests, which can be retired in a follow-up.

Testing Instructions

  • Run the three new suites:
npm test -- src/components/tests/content-tab-settings.actions.test.tsx src/modules/add-site/tests/create-site.actions.test.tsx src/modules/site-settings/tests/edit-site-details.actions.test.tsx
  • Confirm all tests pass
  • Run npm run typecheck and confirm there are no errors

Regression validation

I confirmed that these tests fail on a real regression and that they catch the same bug as the existing e2e suite. I made this change ran both suites:

diff --git a/apps/studio/src/hooks/use-delete-site.ts b/apps/studio/src/hooks/use-delete-site.ts
index 7cc117a1c..33055caa8 100644
--- a/apps/studio/src/hooks/use-delete-site.ts
+++ b/apps/studio/src/hooks/use-delete-site.ts
@@ -33,7 +33,7 @@ export function useDeleteSite() {

                if ( response === DELETE_BUTTON_INDEX ) {
                        try {
-                               await deleteSite( siteId, checkboxChecked );
+                               await deleteSite( siteId, ! checkboxChecked );
                        } catch ( error ) {
                                ipcApi.showErrorMessageBox( {
                                        title: __( 'Deletion failed' ),
EOF
Suite Bug introduced (inverted delete-files flag)
Old e2e (sites.test.ts) CleanShot 2026-06-25 at 11 28 34@2x
New UI integration tests CleanShot 2026-06-25 at 11 10 14@2x

The existing unit tests mock the hook→IPC translation layer, so these aren't catching the introduced issue, only the existing e2e tests and the newly added integration tests are.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

@gcsecsey gcsecsey changed the title Add Site Management UI integration tests (STU-1867) Add Site Management UI integration tests Jun 24, 2026
@gcsecsey gcsecsey requested review from Copilot and gavande1 June 24, 2026 21:10

Copilot AI left a comment

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.

Pull request overview

Adds fast jsdom-based UI integration tests for Studio’s Site Management flows by mounting real renderer components/providers and asserting the IPC commands emitted for each user action (supporting the CLI/UI split testing strategy for STU-1867).

Changes:

  • Added UI action tests for creating sites (suggested/custom name, default/custom path, custom domain + HTTPS) asserting getIpcApi().createSite(...) calls.
  • Added UI action tests for editing site details (rename + PHP version change) asserting getIpcApi().updateSite(...) calls.
  • Added UI action tests for duplicate and delete flows asserting getIpcApi().copySite(...) / getIpcApi().deleteSite(...) calls.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
apps/studio/src/modules/site-settings/tests/edit-site-details.actions.test.tsx New integration-style tests asserting IPC calls for edit-site save + PHP version change.
apps/studio/src/modules/add-site/tests/create-site.actions.test.tsx New integration-style tests asserting IPC calls for multiple site creation variants.
apps/studio/src/components/tests/content-tab-settings.actions.test.tsx New integration-style tests asserting IPC calls for duplicate + delete behaviors from the settings tab.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@gavande1

gavande1 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Great work @gcsecsey, thank you for working on this. I like the overall approach and direction.

One suggestion: as a quick proof of concept, it would be nice to intentionally break something in the components and confirm these tests actually fail. That gives us confidence they catch real regressions and are not passing vacuously.

@gcsecsey

Copy link
Copy Markdown
Member Author

Great work @gcsecsey, thank you for working on this. I like the overall approach and direction.

One suggestion: as a quick proof of concept, it would be nice to intentionally break something in the components and confirm these tests actually fail. That gives us confidence they catch real regressions and are not passing vacuously.

Good suggestion, thanks! I added a section on this to the description. I could introduce an error and catch it with both the existing e2e and the new UI tests, but not the existing unit tests. 👍

@gcsecsey gcsecsey marked this pull request as ready for review June 25, 2026 11:34
@gcsecsey gcsecsey requested a review from fredrikekelund June 25, 2026 11:34
@gcsecsey

Copy link
Copy Markdown
Member Author

Hey @fredrikekelund could you also take a look at the approach in this PR and in #3947 and let us know what you think? Thanks!

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.

3 participants