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

Permalinks in multiple configurations #75

Merged
merged 3 commits into from
Feb 24, 2022

Conversation

cadic
Copy link
Contributor

@cadic cadic commented Feb 22, 2022

Description of the Change

Added comment about file permissions and permalinks in non-default WordPress core configurations.

@cadic cadic requested a review from a team February 22, 2022 11:54
@cadic cadic self-assigned this Feb 22, 2022
@cadic cadic requested review from iamdharmesh and removed request for a team February 22, 2022 11:54
@@ -45,6 +45,10 @@ cy.visit( `/wp-admin` );

The purpose of E2E testing is to ensure the user-facing features work as expected. In the WordPress context, we can extend that purpose to "working as expected against supported WP versions and plugins/themes". At 10up, we're using GitHub Actions matrix and `wp-env` config override to solve that problem by [generating `wp-env` config](https://github.com/10up/simple-podcasting/blob/develop/tests/bin/set-core-version.js) for [each matrix](https://github.com/10up/simple-podcasting/blob/develop/.github/workflows/test-branch.yml#L30-L31). We're doing it for WP core version only, but it can be adapted and updated to handle more complex configurations.

### Permalinks in multiple configurations
Copy link
Contributor

Choose a reason for hiding this comment

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

@cadic We're having the permission issue with the custom WordPress versions rather than multiple configurations. Even if we test for a single configuration, we still have the permission issue.

Maybe something like Fixing the permission issue for custom WordPress versions or Fixing permalink issue for custom WordPress versions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dinhtungdu thank you, I agree the title needs update. I also removed reference to "WordPress core rather than latest" because looks like the issue is now happening on the default core as well: https://github.com/10up/restricted-site-access/actions/runs/1818649424


When testing against non-default (latest) WordPress core, there is [known issue with file permissions](https://github.com/WordPress/gutenberg/issues/28201). This prevents us to use permalinks in testing because `.htaccess` file could not be created in GitHub Actions environment. File permissions could be fixed with [`npm run wp-env run tests-wordpress "chmod -c ugo+w /var/www/html"`](https://github.com/10up/ads-txt/pull/84/files) during the initialization.
There is [known issue with file permissions](https://github.com/WordPress/gutenberg/issues/28201). This prevents us to use permalinks in testing because `.htaccess` file could not be created in GitHub Actions environment. File permissions could be fixed with [`npm run wp-env run tests-wordpress "chmod -c ugo+w /var/www/html"`](https://github.com/10up/ads-txt/pull/84/files) during the initialization.
Copy link
Contributor

Choose a reason for hiding this comment

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

@cadic Actually, I think the previous description is more correct.

WordPress/gutenberg#28201 isn't really related to the current issue because it doesn't mention non-default WordPress versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dinhtungdu in my previous comment a link to RSA Actions log describes that default WordPress core is affected too.

Copy link
Contributor

@dinhtungdu dinhtungdu left a comment

Choose a reason for hiding this comment

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

Just some small typo, but this is LGTM :shipit:

_includes/markdown/Testing.md Outdated Show resolved Hide resolved
Co-authored-by: Tung Du <tung.du@10up.com>
@jeffpaul jeffpaul merged commit 6a40ad8 into gh-pages Feb 24, 2022
@jeffpaul jeffpaul deleted the update/testing-cypress-permalinks branch June 29, 2022 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants