-
Notifications
You must be signed in to change notification settings - Fork 5
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
Get subdomain multisite tests working properly #148
Merged
Merged
Conversation
This file contains 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
Hi from your friendly robot! 🤖 I fixed PHPCS issues with |
pwtyler
approved these changes
Jul 31, 2024
playwright runs on the ci, not on the fixture
this shouldn't need to be done every time, so we check if multisite is true and add the config if it's not true
Co-authored-by: Phil Tyler <[email protected]>
playwright runs on the ci, not on the fixture
jazzsequence
force-pushed
the
add-missing-subdomain-test-step
branch
from
July 31, 2024 20:38
5a860dd
to
dffdcfb
Compare
we can't check the config if it hasn't finished being updated
we should know it already is multisite so we'll just check that the domains exist
so we can pass different values in
we can use this to fix the subdirectory test, too, but later
this is getting reset, too, for some reason
@pwtyler re-requesting review since I've made changes since your ✅ |
jazzsequence
changed the title
add copying the changes in the PR to the subdomain tests
Get subdomain multisite tests working properly
Aug 1, 2024
pwtyler
approved these changes
Aug 1, 2024
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
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.
We're not running the full setup-playwright-tests script because subdomain multisites don't need most of the setup. However, it does need some of the setup (specifically copying the changes from the PR to the fixture site), so we need to manually add those back into that workflow.
In the future we might be able to run just portions of the script for the purpose of subdomain multisite so we're not duplicating code here.
This PR is blocked by #147 -- the changes the robot fixed were part of what was updated in that PR. Merging this PR before that one will create a commit that gets dropped during deploy.This change includes some tweaks to the actual playwright tests and some deviations in the setup. It was discovered in running the tests that the subdomain fixture site was getting reset (GraphQL plugin being deleted, multisite config resetting) -- those have been added to the setup -- but in troubleshooting those it was hypothesized that an incorrect GraphQL endpoint url could have been the issue (it wasn't, multisite not being set up was the issue), or that the environment variables weren't being processed correctly (that wasn't it either, but that was only discovered after breaking the Playwright tests into separate tests -- one for the main site and one for the subdomain). An additional passed environment variable is now supported, however, for GraphQL endpoint, which means that we can eventually remove the handling in the test for different endpoint urls for subdirectory multisite (not included in this PR as it doesn't relate to the subdomain testing).