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

Use actual WordPress in Playground tests #463

Closed
eliot-akira opened this issue May 28, 2023 · 5 comments
Closed

Use actual WordPress in Playground tests #463

eliot-akira opened this issue May 28, 2023 · 5 comments
Labels
Tests [Type] Enhancement New feature or request [Type] Exploration An exploration that may or may not result in mergable code

Comments

@eliot-akira
Copy link
Collaborator

From discussion in #427 comment.

wp-now could be useful for server-side tests to make use of a full WordPress environment

A difficulty: if wp-now imports from the Playground package being tested, the tests cannot import from wp-now because it results in a circular dependency.

When I imported the package from the test, I got an error from nx or vitest, "Could not execute command because the task graph has a circular dependency" - because that package imports blueprints.

That’s a challenging one. Perhaps building wp-now to a single bundle and then using that to run Blueprints tests would solve it here. This is the second time this comes up today- the first time was in #434

Found a documentation page about this in the nx repository.

Resolving Circular Dependencies

Strategies recommended:

  • Make more granular projects with less inter-dependence
  • Or merge such projects into a single project that is free to depend on itself
  • Or separate the extra logic into a new project that is shared between the projects which desire this logic
@adamziel
Copy link
Collaborator

adamziel commented May 28, 2023

@eliot-akira I’m thinking of using the WordPress data modules that already come with the repository. Perhaps a @wp-playground/wordpress package would make sense for those? Not sure yet.

About wp-now specifically, I will propose a vision and a crisp scope for the core Playground repo in a few days. The idea is to stabilize the project, have predictable expectations, and keep it focused on the framework. This involves moving consumer apps to separate repos so let’s put the wp-now part of this issue on hold for now.

@adamziel adamziel added [Type] Enhancement New feature or request Tests labels May 28, 2023
@adamziel
Copy link
Collaborator

adamziel commented Jun 2, 2023

With wp-now moved to the playground-tools repo, let's find a way to resolve this using the building blocks available in this repository.

@adamziel adamziel changed the title How to use wp-now package from Playground tests Use actual WordPress in Playground tests Jun 2, 2023
@adamziel adamziel added the [Type] Exploration An exploration that may or may not result in mergable code label Jun 2, 2023
@adamziel
Copy link
Collaborator

adamziel commented Nov 30, 2023

There's a precedent of this in sql.spec.ts:

async function getWordPressDataModule() {
const wpData = readFileSync(__dirname + '/wp-6.3.data');
const wpDataArrayBuffer = wpData.buffer.slice(
wpData.byteOffset,
wpData.byteOffset + wpData.byteLength
);
shimXHR(wpDataArrayBuffer);
// @ts-ignore
return await import('./wp-6.3.js');
}

We should find a way to generalize this to all packages. I wonder if Blueprints should do that internally as Playground depends on Blueprints. If Blueprints also depended on Playground, that would be a circular dependency. We should be fine to just copy WordPress 6.3 data module to Blueprints for now and then reflect on the internal dependency graph. Perhaps introducing a playground-test-utils package would solve this?

@eliot-akira
Copy link
Collaborator Author

Perhaps introducing a playground-test-utils package would solve this?

That sounds like a good solution, it aligns with one of the suggestions in the nx repo's Wiki (here).

Another common case is that the consumer of a project has some extra logic that is desirable from producing libraries. When this extra logic is consumed by a producing project, it creates a circular dependency. The solution to this case is to separate the extra logic in the producing project into a new project that is shared between the projects which desire this logic.


We should be fine to just copy WordPress 6.3 data module to Blueprints for now

I saw that the new package playground/sync bundles its own copy of wp-6.3.{js,wasm} in src/test. Wouldn't it be better to use those same files from the php-wasm package (in node/public) since they're already checked into the repo?

adamziel added a commit that referenced this issue Dec 5, 2023
#837)

Moves WordPress data modules to a new dependency-free module
`@wp-playground/wordpress`. Now every Playground module may import
WordPress data modules and, e.g., use them in unit tests.

Before this PR, WordPress data modules lived in the
`@wp-playground/remote` package. They couldn't be easily imported, and
even if they could, that would often create a circular dependency
problem.

Related to #463

## Implementation details

* All WordPress bundling functions are updated to point to the newly
created module.
* The `@wp-playground/remote` public directory is now set to
`packages/playground/wordpress/public` as that's where all the WordPress
files live. The `remote` package ships no public assets of its own,
except for the .htaccess file which is now handled by a dedicated
plugin.
* The new module also exports `RecommendedPHPVersion = '8.0'` to help
avoid hardcoding a version number in unit tests shipped by other
Playground packages.
* `@wp-playground/wordpress` has a `bundle-wordpress` nx task that
bundles WordPress. It's deliberately not called `build` so that nx does
not mistake it for a build dependency (which would trigger it on every
`npm run dev`).
* Loading the data module from the filesystem is implemented in
`@php-wasm/node` by implementing the `getPreloadedPackage` Emscripten
handler. The handler runs a few safety checks and then reads the data
module from the disk via `readFileSync`.
* A new unit test for the `setSiteOptions()` Blueprint step has been
added to confirm that WordPress is actually correctly loaded in Node.js.

## Remaining tasks

- [x] Run the new `bundle` task to confirm it creates the correct
WordPress files in the correct directories.
- [x] Add a unit test to the new module to explicitly test loading
WordPress (instead of implicitly relying on the `setSiteOption` test)

## Testing instructions

Confirm the CI tests pass. E2E tests will confirm the
playground.wordpress.net website still loads all the supported WordPress
versions, and the unit tests will confirm that WordPress is correctly
loaded in Node.js.
@adamziel
Copy link
Collaborator

adamziel commented Jan 31, 2024

With Node.js WordPress support initially merged in #837 and a battery of WordPress-related unit tests added in #982, this issue seems to be solved.

Here's a unit test covering the activateTheme Blueprint step by interacting with an actual WordPress module sourced from the same .zip file as used by playground.wordpress.net website (the .data modules are gone since #978):

https://github.com/WordPress/wordpress-playground/blob/trunk/packages/playground/blueprints/src/lib/steps/activate-theme.spec.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tests [Type] Enhancement New feature or request [Type] Exploration An exploration that may or may not result in mergable code
Projects
None yet
Development

No branches or pull requests

2 participants