Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CMSP-1140] add Playwright tests for multisite #140

Merged
merged 112 commits into from
Jul 18, 2024

Conversation

jazzsequence
Copy link
Contributor

@jazzsequence jazzsequence commented Jul 11, 2024

This PR adds Playwright tests for multisite.
This aims to help us identify problems in the upstream before they are shipped.

Included in this PR:

  • subdomain and subdirectory variants of application.php
  • an updated wpcm.spec.ts test suite that takes parameters from the environment to run tests
  • an updated playwright.yml with added jobs for subdirectory and subdomain multisite tests
  • devops/scripts/setup-playwright-tests.sh -- a script that standardizes the setup process for single WP sites and subdirectory multisites (because subdomain multisites need to have a site plan, the fixture site is not destroyed after the tests are run)
  • some minor changes to filters.php that were done at the same time as testing. there is nothing significant here, some standardization for making fix_core_resource_urls multipurpose and whitespace fixes

Single site and subdirectory multisite tests take ~10m to complete when they have not previously failed. The script is designed to skip some setup steps if the site was not deleted and the site is only deleted if the tests pass.

Subdomain multisite tests take ~2m to complete because we don't need to run the setup.

we can copy this pattern for subdomain and subdirectory multisites
maybe remove this from composite?
so they're easier to read in the action workflow
instead of the upstream name
this isn't relevant anymore
set the var to the id 🤦‍♂️
.github/fixtures/config/application.subdir.php Outdated Show resolved Hide resolved
Comment on lines 13 to 20
test("WP REST API is accessible", async ({ request }) => {
const apiRoot = await request.get(`${siteUrl}/wp-json`);
let apiRoot = await request.get(`${siteUrl}/wp-json`);
// If the api endpoint isn't /wp-json, it should be /wp/wp-json. This will probably be the case for main sites on subdirectory multisites.
if (!apiRoot.ok()) {
apiRoot = await request.get(`${siteUrl}/wp/wp-json`);
}
expect(apiRoot.ok()).toBeTruthy();
});
Copy link
Member

Choose a reason for hiding this comment

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

Don't love the makeup of this test and next diff, we should be testing one or the other and know when we're going to get one or the other, not check .ok()

That said, care 3/10

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only time this comes into play is in subdirectory multisite on the main site. The paths for graphql and wp-api are /wp/graphql and /wp/wp-json in either of those cases. I fought to try to drop the /wp but eventually gave up. It might be a core multisite thing actually.

This check and the switching of the URLs is for when we're testing on the main site of a subdirectory multisite vs testing any of the subsites or subdomain multisite. We could go a step further and confirm that somehow (but I'm not sure how to do that from the outside with javascript).

devops/scripts/setup-playwright-tests.sh Outdated Show resolved Hide resolved
devops/scripts/setup-playwright-tests.sh Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

broadly feels like we should either have functions accepting parameters, OR a single main() function, but acceptable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about rewriting the whole thing with a main function and making all the steps in the GHA use the script and call the respective parts, but that was towards the end of this process and I was ready to be done.

devops/scripts/setup-playwright-tests.sh Outdated Show resolved Hide resolved
devops/scripts/setup-playwright-tests.sh Outdated Show resolved Hide resolved
devops/scripts/setup-playwright-tests.sh Outdated Show resolved Hide resolved
devops/scripts/setup-playwright-tests.sh Outdated Show resolved Hide resolved
devops/scripts/setup-playwright-tests.sh Outdated Show resolved Hide resolved
jazzsequence and others added 16 commits July 17, 2024 15:09
move cookie settings down so we can use pantheon hostname
Co-authored-by: Phil Tyler <[email protected]>
TIL ✨

Co-authored-by: Phil Tyler <[email protected]>
don't exit because that bails the script

Co-authored-by: Phil Tyler <[email protected]>
Co-authored-by: Phil Tyler <[email protected]>
Co-authored-by: Phil Tyler <[email protected]>
…on-systems/wordpress-composer-managed into cmsp-1140-add-tests-for-multisite
this won't catch everything but maybe it'll catch what's breaking things now
@jazzsequence
Copy link
Contributor Author

had to make some tweaks to account for when commit messages have apostrophes, apparently....

@jazzsequence jazzsequence requested a review from pwtyler July 17, 2024 21:56
@jazzsequence jazzsequence merged commit 03c8f04 into default Jul 18, 2024
6 checks passed
@jazzsequence jazzsequence deleted the cmsp-1140-add-tests-for-multisite branch July 18, 2024 20:48
jazzsequence added a commit that referenced this pull request Jul 31, 2024
* abstract out the site_id and pass a site url to playwright
we can copy this pattern for subdomain and subdirectory multisites

* allow site name to come from environment

* move playwright tests to repeatable composate action

* run actions/checkout first
maybe remove this from composite?

* remove checkout from composite action

* Update playwright.yml to use the correct path for run-playwright-tests

* pass all the parameters from the workflow that has that info

* add shell

* add logging

* log the deletion, too

* pass the site name to the playwright tests

* fix broken colors

* add stdbuf to commands to log everything to the terminal

* shorten the names
so they're easier to read in the action workflow

* set the upstream id

* reference the action file directly

* create application.php files for subdir & subdom multisite

* copy the config file to the config/ directory

* absolutely do not use the action.yml path

* fix linting in two new files

* just use actual upstream IDs
instead of the upstream name

* remove comment
this isn't relevant anymore

* don't set the ID to the new ID
set the var to the id 🤦‍♂️

* check the multisite install when we status check

* fix broken conditional

* the conditional needs to wrap around the whole multisite block

* add a db reset before site install

* add the updated application.php file before the multisite check

* make sure we're in the right directory

* rename the file

* be specific about the type of multisite in the commit message

* move the wait to the top
so we can wait for the previous commit to finish

* use the correct site url (not the copy pasta one)

* display art for no good reason

* wait for the config to hit the server
because otherwise we're checking for the install status too early

* yeah 🤦‍♂️

* I guess we just can't do a colored logo

* set a max wait on the wait before installing graphql
because if a process is running, it'll wait longer, but if a process isn't (like we already waited earlier) then it's just going to use the long timeout

* only add the subsite if it doesn't already exist

* split the clone and copy updates apart
and put the multisite config in the middle
this way we're only waiting for sync_code once

* we do actually need the github token

* fix typo

* don't fail if there's nothing to commit

* just see if a foo site exists

* break the composite workflow into a script

* PRIVATE_KEY not SSH_KEY

* fix terminus token, too

* set defaults & move functions to the bottom

* shellcheck

* fix the other places inputs were broken

* fix secret name

* delete the sites when we're done

* remove any exits that aren't errors

* fix file copy globbing

* remove composite action

* move install dependencies into action & pantheon host key into script

* get last commit message and export to the environment

* remove host key command
we're still doing it in the action and it works for single site

* use the github token

* cd to the workspace

* need to get PR number

* echo the commit message for debugging

* use upstream name instead of id

* move playwright tests into the main workflow

* remove the blue var
unused

* need to install deps in single site

* multisite doesn't add host key
copy/pasta the two setups around keys and terminus so it's consistent

* rename run-playwright-tests to setup-

* Commit GraphQL to the repo

* move the workflow:wait to after we copy updates
because we're, again, checking multisite config before the commit is there

* use test instead of comparing value
i think this is failing because it's returning empty when it's not set

* don't use test, just run the command

* set connection to git always

* store the value of the config get in a var and check that

* install wp before doing status check

* fix subdir siet url vars

* force db reset

* the script shouldn't fail if foo site isn't found

* fix site title

* store site name and url in Github env

* normalize rest endpoint for multisite

* separately apply the filter to rest urls

* filter the rest endpoint, too

* remove all the wp-json handling

* adjust the playwright test to expect the api endpoint to be /wp/wp-json

* change the site name before we test the subsite

* the graphql endpoint changes if we're on a subsite, too

* flush the cache for the sub-site if we're doing multisite

* fix the toBeTruthy check

* echo with -e for color

* fix spacing

* only log in if not already logged in
allows you to run this locally with existing terminus token

* don't flush permalinks here
they're flushed after the site is created

* add tests for subdomain

* fix the subdom site url

* [CMSP-1140] Move cookie settings down to use Pantheon hostname (#141)

move cookie settings down so we can use pantheon hostname

* wrap PANTHEON_ENVIRONMENT with an isset

* remove the comment

Co-authored-by: Phil Tyler <[email protected]>

* make the globals readonly

TIL ✨

Co-authored-by: Phil Tyler <[email protected]>

* return early

don't exit because that bails the script

Co-authored-by: Phil Tyler <[email protected]>

* use `[[` instead of `[`

Co-authored-by: Phil Tyler <[email protected]>

* return early

Co-authored-by: Phil Tyler <[email protected]>

* one terminus command to activate wp-graphql

Co-authored-by: Phil Tyler <[email protected]>

* use brackets around commit msg

* use printf to handle single quotes in commit messages

* skip creating a new variable if we do not need it

Co-authored-by: Phil Tyler <[email protected]>

* strip out single quotes
this won't catch everything but maybe it'll catch what's breaking things now

* better handling of sed

* use printf

* strip newlines and carriage returns

* better commit message cleaning

---------

Co-authored-by: Phil Tyler <[email protected]>
jazzsequence added a commit that referenced this pull request Aug 1, 2024
* abstract out the site_id and pass a site url to playwright
we can copy this pattern for subdomain and subdirectory multisites

* allow site name to come from environment

* move playwright tests to repeatable composate action

* run actions/checkout first
maybe remove this from composite?

* remove checkout from composite action

* Update playwright.yml to use the correct path for run-playwright-tests

* pass all the parameters from the workflow that has that info

* add shell

* add logging

* log the deletion, too

* pass the site name to the playwright tests

* fix broken colors

* add stdbuf to commands to log everything to the terminal

* shorten the names
so they're easier to read in the action workflow

* set the upstream id

* reference the action file directly

* create application.php files for subdir & subdom multisite

* copy the config file to the config/ directory

* absolutely do not use the action.yml path

* fix linting in two new files

* just use actual upstream IDs
instead of the upstream name

* remove comment
this isn't relevant anymore

* don't set the ID to the new ID
set the var to the id 🤦‍♂️

* check the multisite install when we status check

* fix broken conditional

* the conditional needs to wrap around the whole multisite block

* add a db reset before site install

* add the updated application.php file before the multisite check

* make sure we're in the right directory

* rename the file

* be specific about the type of multisite in the commit message

* move the wait to the top
so we can wait for the previous commit to finish

* use the correct site url (not the copy pasta one)

* display art for no good reason

* wait for the config to hit the server
because otherwise we're checking for the install status too early

* yeah 🤦‍♂️

* I guess we just can't do a colored logo

* set a max wait on the wait before installing graphql
because if a process is running, it'll wait longer, but if a process isn't (like we already waited earlier) then it's just going to use the long timeout

* only add the subsite if it doesn't already exist

* split the clone and copy updates apart
and put the multisite config in the middle
this way we're only waiting for sync_code once

* we do actually need the github token

* fix typo

* don't fail if there's nothing to commit

* just see if a foo site exists

* break the composite workflow into a script

* PRIVATE_KEY not SSH_KEY

* fix terminus token, too

* set defaults & move functions to the bottom

* shellcheck

* fix the other places inputs were broken

* fix secret name

* delete the sites when we're done

* remove any exits that aren't errors

* fix file copy globbing

* remove composite action

* move install dependencies into action & pantheon host key into script

* get last commit message and export to the environment

* remove host key command
we're still doing it in the action and it works for single site

* use the github token

* cd to the workspace

* need to get PR number

* echo the commit message for debugging

* use upstream name instead of id

* move playwright tests into the main workflow

* remove the blue var
unused

* need to install deps in single site

* multisite doesn't add host key
copy/pasta the two setups around keys and terminus so it's consistent

* rename run-playwright-tests to setup-

* Commit GraphQL to the repo

* move the workflow:wait to after we copy updates
because we're, again, checking multisite config before the commit is there

* use test instead of comparing value
i think this is failing because it's returning empty when it's not set

* don't use test, just run the command

* set connection to git always

* store the value of the config get in a var and check that

* install wp before doing status check

* fix subdir siet url vars

* force db reset

* the script shouldn't fail if foo site isn't found

* fix site title

* store site name and url in Github env

* normalize rest endpoint for multisite

* separately apply the filter to rest urls

* filter the rest endpoint, too

* remove all the wp-json handling

* adjust the playwright test to expect the api endpoint to be /wp/wp-json

* change the site name before we test the subsite

* the graphql endpoint changes if we're on a subsite, too

* flush the cache for the sub-site if we're doing multisite

* fix the toBeTruthy check

* echo with -e for color

* fix spacing

* only log in if not already logged in
allows you to run this locally with existing terminus token

* don't flush permalinks here
they're flushed after the site is created

* add tests for subdomain

* fix the subdom site url

* [CMSP-1140] Move cookie settings down to use Pantheon hostname (#141)

move cookie settings down so we can use pantheon hostname

* wrap PANTHEON_ENVIRONMENT with an isset

* remove the comment

Co-authored-by: Phil Tyler <[email protected]>

* make the globals readonly

TIL ✨

Co-authored-by: Phil Tyler <[email protected]>

* return early

don't exit because that bails the script

Co-authored-by: Phil Tyler <[email protected]>

* use `[[` instead of `[`

Co-authored-by: Phil Tyler <[email protected]>

* return early

Co-authored-by: Phil Tyler <[email protected]>

* one terminus command to activate wp-graphql

Co-authored-by: Phil Tyler <[email protected]>

* use brackets around commit msg

* use printf to handle single quotes in commit messages

* skip creating a new variable if we do not need it

Co-authored-by: Phil Tyler <[email protected]>

* strip out single quotes
this won't catch everything but maybe it'll catch what's breaking things now

* better handling of sed

* use printf

* strip newlines and carriage returns

* better commit message cleaning

---------

Co-authored-by: Phil Tyler <[email protected]>
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