Skip to content

Conversation

@adamziel
Copy link
Collaborator

@adamziel adamziel commented May 30, 2025

Motivation for the change, related issues

Supersedes #2221 by using a merge commit with two parents and cherry-picking everything that wasn't already present in the private branch. Let's see if the tests pass here

Implementation details

Testing Instructions (or ideally a Blueprint)

bgrgicak and others added 30 commits January 20, 2025 19:53
…the dirname plugin during build (#4)

## Motivation for the change, related issues

The PHP dependency path produced by the PHP-wasm Node build assumes that
`__dirname` ends with a trailing slash while it ends without it
[(Node.js __dirname
documentation).](https://nodejs.org/api/modules.html#__dirname)
This results in an incorrect path to the PHP js and wasm files, for
example `/usr/testjspi/8_0_30/php_8_0.js` while it should be
`/usr/test/jspi/8_0_30/php_8_0.js`.

These incorrect paths prevent Playground from booting when we run it
using Node with an `Error: ENOENT: no such file or directory, open`
error.

To ensure the path to the PHP-wasm dependencies is correctly built, this
PR adds a slash between `__dirname` and the appended path
(_jspi_/_asyncify_).

Fixes #2129

## Testing Instructions (or ideally a Blueprint)

- Checkout `trunk`
- Build PHP-wasm Node `npx nx build php-wasm-node`
- Confirm that the paths produced by the build are missing a slash
before the _jspi_ and _asyncify_ keyword
```
grep '_dirname +' dist/packages/php-wasm/node/index.cjs
```
- Checkout this branch
- Build PHP-wasm Node `npx nx build php-wasm-node`
- Confirm that the paths produced by the build have a slash before the
_jspi_ and _asyncify_ keyword
```
grep '_dirname +' dist/packages/php-wasm/node/index.cjs
```

Co-authored-by: Bero <[email protected]>
## Motivation for the change, related issues

Note: This work is continued from a PR started under the WordPress GH
org.

There are a couple of reasons for this upgrade:

1. I am having trouble rebuilding php-wasm on my current system.
References to older ubuntu images and emscripten versions [has given me
trouble](#2038 (comment)).
2. We want [this Emscripten
fix](emscripten-core/emscripten#22932) from
@jeroenpf and @bgrgicak which was first included in Emscripten 3.1.73.
This should address
https://github.com/Automattic/dotcom-forge/issues/8815.

## Implementation details

This PR:
- Bumps the ubuntu docker image version due to build errors related to
missing image. Referencing a newer ubuntu image seemed to fix the
"missing image" errors.
- Bumps Emscripten version
- Adjusts Playground-specific patches to Emscripten output since the
latest breaks that patching.

## Testing Instructions (or ideally a Blueprint)

- CI (once we can run GH actions on A8C's GHE instance)

---------

Co-authored-by: Brandon Payton <[email protected]>
## Motivation for the change, related issues

During the dotorg pause, we need a way to privately publish JS package
updates.

## Implementation details

This PR adds an nx executor for creating packages that can be served via
HTTP and referenced in package.json via URL.

It takes a base URL for hosted packages (`hostingBaseUrl`) and creates
package tarballs like `@php-wasm-node-1.0.21.tar.gz`. Each package's
tarball contains a package.json with Playground dependencies updated to
reference the same base URL.

For example, if the package.json for `@php-wasm/node` has a dependency
like
```json
"@php-wasm/universal": "1.0.21",
```
the package.json in its tarball will include a dependency like:
```json
"@php-wasm/universal": "<base-url>/@php-wasm-universal-1.0.21.tar.gz",
```

After running the `package-for-self-hosting` executor for a given base
URL, there should be a tarball for each built package under
`dist/packages-for-self-hosting/<base-url>`.

## Testing Instructions (or ideally a Blueprint)

From the root of a local Playground clone:
- Run `npx nx run-many --all --target=package-for-self-hosting --
--hostingBaseUrl=http://127.0.0.1:6789/`
- Confirm the build completes successfully
- `cd 'dist/packages-for-self-hosting/http%3A%2F%2F127.0.0.1%3A6789%2F'`
- Run `php -S 127.0.0.1:6789 -t .` to start a local web server
- In another terminal:
    - Create a temp directory for testing
- Run `npm init` and take all the default options to create a dummy
package
- Run `npm install
http://127.0.0.1:6789/@wp-playground-cli-1.0.21.tar.gz`
- Run `npx cli server` and confirm Playground CLI successfully started a
server
- cd into the node_modules directory, examine multiple package.json
files under `@php-wasm` and `@wp-playground` scopes and confirm their
playground-related deps all reference tarball URLs rather than npm
package versions.
## Motivation for the change, related issues

We are doing some private releases to be used by the Studio app but want
to release the source in respect of the GPL license.

## Implementation details

This PR:
- Updates `.gitattributes` to avoid exporting large, non-source files
via `git archive`. This change reduced the size of the source package
from 810MB to 81MB (oddly by an exact factor of 10).
- Adds a script for running `git archive` and manually adding a couple
of supporting items like the `isomorphic-git` submodule and build
instructions for the source package.

## Testing Instructions (or ideally a Blueprint)

From the project root, run:
`./tools/scripts/package-source.sh <branch-name> <output-tarball-path>`

Find a place to unpack the tarball, examine it, open
`BUILD-FROM-SOURCE-RELEASE.md`, and follow the instructions to rebuild
php-wasm and wordpress-builds. This will likely take a long time. But
once you've done the setup, it should be possible to build specific
packages we release to NPM using `npx nx` commands.
## Motivation for the change, related issues

There are some screenshot-related e2e failures here:

https://github.com/Automattic/wordpress-playground-private/actions/runs/13023102742/job/36327885608#step:7:1744

## Implementation details

It turns out the "this is experimental" notice was [removed in
September](39d0023#diff-6d0121c3b993ae42d239b69beb19655bd0cb7ac46fe300d8791294d839b86addL98-L100),
and the reference screenshots were apparently not different enough to
cause an error. But now we're seeing those errors for Firefox and
WebKit. The Chromium screenshot still has the notice but is apparently
not triggering an error yet.

For now this just updates the breaking reference screenshots.

## Testing Instructions (or ideally a Blueprint)

- CI
## Motivation for the change, related issues

We need to deploy JS packages to a public-but-self-hosted location so
the Studio app can consume as dependencies. We expect this to continue
until we are again publishing releases to the NPM repo.

## Implementation details

Add a workflow for deploying self-hosted packages.

## Testing Instructions (or ideally a Blueprint)

We cannot test by manually running a new workflow until a version of the
workflow exists on the main branch. So we will have to commit this PR
without testing the workflow and follow up with a PR to fix the
workflow.

We can the next PR like this:
- Manually run the new workflow
- SSH into our package server and confirm the packages are deployed as
expected
- Create a local node package
- Add a dependency on one of the self-hosted packages that depends on
another (the `@wp-playground/cli` package URL should be good)
- `npm install`
- Confirm installation worked by testing `npx cli server`
## Motivation for the change, related issues

Fixes #10

Playground CLI package builds currently do not run properly due to
missing `readline` external.

## Implementation details

This PR declares the readline module as an external in `vite.config.js`.

## Testing Instructions (or ideally a Blueprint)

- Tested and examined build output to note proper import of 'readline'
module.
- Will confirm fix under PR #6 because that is where I first discovered
the issue.
## Motivation for the change, related issues

This PR follows up on #5 which added an untested self-hosted packages
workflow.

## Implementation details

This PR will contain a variety of fixes depending on the bugs discovered
when manually running the self-hosted package workflow.

## Testing Instructions (or ideally a Blueprint)

- Manually run the new workflow
- SSH into our package server and confirm the packages are deployed as
expected
- Create a local node package
- Add a dependency on one of the self-hosted packages that depends on
another (the `@wp-playground/cli` package URL should be good)
- `npm install`
- Confirm installation worked by testing `npx cli server`
## Motivation for the change, related issues
Fixes
Automattic/wordpress-playground-private#12
Integrate CLI with `wp-now` features. This PR adds a `--launch-browser`
flag to CLI, default false.

## Implementation details
It backport the `openInDefaultBrowser` function found in wp-now to CLI
`startServer`.

## Testing Instructions (or ideally a Blueprint)
Run the script as `bun packages/playground/cli/src/cli.ts server
--launch-browser` or similar. The flag should start the default browser.
## Motivation for the change, related issues
Files in a zip file should only have relative urls as per spec. Right
now, when exporting the backup for the site, we create a zip file with a
leading slash in the files inside it.

This got surfaced in Studio app when we switched to a diff zip library
that strictly adheres to zip spec and PG exported zip files fail to
process.

Related to: Automattic/dotcom-forge#10369

Note: There are other places where `ZipArchive` is used but this PR only
modifies the location that impacts the site export.
<img width="531" alt="Screenshot 2025-01-30 at 11 44 32"
src="https://github.com/user-attachments/assets/561896ee-0fe9-40c1-b413-d226c68fc0be"
/>
Probably other places can use the same change, and I can follow that up
in a separate PR, if you like.

## Testing Instructions (or ideally a Blueprint)
Export zip of a site and examine the file paths inside it using the
following script:

```php
<?php
$zip = new ZipArchive();
$zip->open('/Users/ashfame/Downloads/wordpress-playground-16.zip');
echo "=== ZIP Contents ===".PHP_EOL;
for($i = 0; $i < $zip->numFiles; $i++) {
	echo $zip->getNameIndex($i) . PHP_EOL;
}
echo "=== End ZIP Contents ===".PHP_EOL;
$zip->close();
```

Prior to this PR, you would see file paths like:
```
/wp-content/themes/twentytwentyfive/patterns/more-posts.php
```
and with this PR you will see file paths without the leading slash:
```
wp-content/themes/twentytwentyfive/patterns/more-posts.php
```
## Motivation for the change, related issues

@ashfame is working on Playground for Studio and needs to be able to
release package updates.

## Implementation details

Add the right username to the release-and-deploy workflow.

## Testing Instructions (or ideally a Blueprint)

This is a simple change, and we will test the next time the workflow is
run.
Reverts Automattic/wordpress-playground-private#18 which adds a
platform-level feature "open in the system browser".

It is a useful feature; it just doesn't seem like a good fit at the
platform level.

There are too many OS and JS runtime combinations that we can reasonably
test. It's easy to imagine reports saying it doesn't work on Node 18 on
Linux Arch, and there's not much value in spending time solving this
type of error. Case in point: the original PR already adds an exception
for GitHub codespaces. We would inevitably see more exceptions and
special rules for other online platforms. Let's not go there at all and
leave it to the API consumers.

Every CLI consumer is free to implement that feature on their own or use
a ready package. Let's leave it to them. For the Playground platform,
it's less about "what can we add" and more about "what can we leave
out?"

cc @bgrgicak @zaerl
## Motivation for the change, related issues

In testing, WP Cloud ssh access does not always play nicely with scp. 

## Implementation details

We just copy a single file with scp, and in manual tests, switching to
rsync avoids the issue.

## Testing Instructions (or ideally a Blueprint)

- Manual testing of commands via command line before merging.
- Running the workflow after merging.
## Motivation for the change, related issues

We need to do some special things to deploy a Playground web app that is
private to Automattic (e.g., adding a check for A8C proxy). Let's
maintain private site deployment as a separate workflow so that those
changes don't eventually propagate to the main website deployment
workflow used by https://github.com/WordPress/wordpress-playground (when
we resume public contributions).

## Implementation details

Fork the website deployment workflow into deploy-private-website.yml. We
could make deployment to WP Cloud a reusable action if we need to deploy
to more sites than this, but let's just copy the workflow for now.

## Testing Instructions (or ideally a Blueprint)

- Deploy initial file and test manually afterward. This is required for
manual testing because you cannot launch a workflow that hasn't yet been
merged to the main branch (AFAIK).
## Motivation for the change, related issues

Some tweaks are needed to the private website deployment workflow, now
that it exists on trunk and can be manually tested.

## Implementation details

- Fix name to say it is for private site deployment
- Fix other issues discovered in testing.

## Testing Instructions (or ideally a Blueprint)

- Manually run the workflow and confirm that it succeeds.
- Try to access the site while unproxied, and confirm you receive an
"Unauthorized. Please proxy" message.
- Try to access the site while proxied, and confirm that Playground
loads successfully.
We have some incorrect screenshots and test instability. This PR fixes
some broken screenshots that I added in a mistaken previous PR and will
experiment with adjusting other settings to get more reliable results.
It adds [Remote Data
Blocks](https://github.com/Automattic/remote-data-blocks) to the allowed
repositories for plugin proxy.

## Motivation for the change, related issues

In [this PR](Automattic/remote-data-blocks#347),
we are attempting to add a live playground link for each PR so that it's
easy to test out the proposed changes. The inspiration for this came
from [the woocommerce repo](https://github.com/woocommerce/woocommerce)
that had the same feature.

## Implementation details

The code change is the same as what was done for woocommerce, and sensei
in order to support the same functionality.

## Testing Instructions (or ideally a Blueprint)

- Create a GitHub token with the "repo" scope checked for testing.
- Add the token hard coded in the
`packages/playground/website/public/plugin-proxy.php`.
- Serve the `wordpress-playground/packages/playground/website/public`
with a PHP server.
- Request

`/plugin-proxy.php?org=Automattic&repo=remote-data-blocks&workflow=Build%20Live%20Branch&artifact=remote-data-blocks-347&pr=347`
- Check that the request works properly.

Note: Depending on if you have permission to generate the right token or
not, the request will fail with a 401. I verified where the failure was
to ensure it actually tried to make the request to the correct repo
rather than rejecting it outright.
## Motivation for the change, related issues

Adding myself to the GitHub workflows actors as per the new team member
onboarding.
## Motivation for the change, related issues

- We want to deploy the private web app regularly.
- Let's use a dedicated environment for private deployment so no private
deployment happens in public repo when/if commits containing this
workflow are merged there (because the named private deployment
environment won't exist).

## Implementation details

- Uncomment the schedule config in the private deploy workflow
- Switch the private deploy workflow to a dedicated environment

## Testing Instructions (or ideally a Blueprint)

- Manually run the updated workflow on this branch and confirm it
succeeds
## Motivation for the change, related issues

We want to host a private Playground instance and are seeking to
eliminate places where `playground.wordpress.net` is hard-coded. It
turns out that URLs within app manifests can be relative, so we can
remove mention of playground.wordpress.net from manifest.json.

## Implementation details

This PR:
- Converts paths in manifest.json to relative paths
- Removes the manifest-related operations in the build step because both
the built website and the dev version (via `npm run dev`) are
installable as PWAs
- Moves `manifest.json` into the website package's `public/` folder so
Vite stops moving and mangling the filename.

## Testing Instructions (or ideally a Blueprint)

- CI

Test built app:
- Run `npx nx build playground-website`
- Run `php -S 127.0.0.1:7778 -t
dist/packages/playground/wasm-wordpress-net` to start a web server
- Visit `https://127.0.0.1:7778` in Chrome and note the site is
installable as a PWA

Test dev:
- Run `npm run dev`
- Visit the dev site in Chrome and note the site is installable as a PWA
## Motivation for the change, related issues
The libsqlite version [specified for PHP WASM
build](https://github.com/Automattic/wordpress-playground-private/blob/e092d448c9cd0fe56cde3a5048c132b3c55b3c04/packages/php-wasm/compile/libsqlite3/Dockerfile#L16)
was not respected for PHP < 7.4. This change fixes it for all PHP
versions.

Fixes
Automattic/wordpress-playground-private#31.

## Implementation details
Originally, it seemed that for PHP < 7.4 this [needs to be done
differently](https://www.php.net/manual/en/ref.pdo-sqlite.php):

> The PDO_SQLITE PDO driver is enabled by default. To disable,
--without-pdo-sqlite[=DIR] may be used, where the optional [=DIR] is the
sqlite base install directory. As of PHP 7.4.0 [»
libsqlite](http://sqlite.org/) ≥ 3.5.0 is required. Formerly, the
bundled libsqlite could have been used instead, and was the default, if
[=DIR] has been omitted.

However, during my testing, it turned out that simply keeping
`-lsqlite3` for all PHP versions fixes the issue. I verified this for
PHP 7.0, 7.1, 7.2, and 7.3.

## Testing Instructions (or ideally a Blueprint)
Run the following command, replacing `7.0` with any desired PHP version:
```js
PHP=7.0 npx nx run php-wasm-cli:start -- -r "var_dump(PHP_VERSION); var_dump((new PDO('sqlite::memory:'))->query('select sqlite_version()')->fetch());"
```
## Motivation for the change, related issues

In #25, we want to ship a wp-cli.phar and wordpress-importer.zip with
the Blueprints library in order to eliminate hard-coded references to
the Playground web app (That way, we can host a private Playground
without reference to the public one). In addition, it's probably better
to just version the binary dependencies along with the library.

But this made existing code and tests awkward:
We want to reference a wp-cli.phar shipped with the Blueprints lib but
need to reference it differently depending on whether the Blueprints lib
is running on the command line or in a browser.

It turns out we can use the same resource type to do both if we base
wp-cli.phar URLs on `import.meta.url`. In a browser environment, it is
an http(s):// URL, and in Node.js, the it is a file:// URL. In both
cases, Vite is able to make sure public asset URLs based on
`import.meta.url` are properly set.

## Implementation details

This PR updates the FetchResource abstract type to be able to resolve
data when the specified URL is a file:// URL and the module's URL is
also a file:// URL (indicating a runtime that is loading from local FS).

## Testing Instructions (or ideally a Blueprint)

- Added test for file URLs.
- CI
## Motivation for the change, related issues

We need to better support self-hosting the Playground web app (as we are
now deploying a private instance), and the current CORS proxy only
supports requests from playground.wordpress.net and localhost.

Let's allow customizing the list of supported origins.

## Implementation details

This PR specifies the current supported origins as the default list and
allows it to be overridden by defining a PHP constant
`PLAYGROUND_CORS_PROXY_SUPPORTED_ORIGINS`.

This PR also stops the web app build and deployment from including its
own CORS proxy. We don't want this in the main, public web app, so let's
just keep the two completely separate.

## Testing Instructions (or ideally a Blueprint)

Test after merge part of a private CORS proxy deployment.
## Motivation for the change, related issues

The original implementation of the DNS polyfill didn't register `DNS_*`
constants which are [expected to exist in
PHP](https://www.php.net/manual/en/network.constants.php#constant.dns-any).
To fix this this PR registered the required DNS constants.

## Implementation details

The constants were already defined in the polyfill and now we register
them as PH constants using `REGISTER_LONG_CONSTANT`.

I didn't want to add more code to the `php_wasm.c` file that wasn't
directly related to PHP-wasm, so I moved the DNS polyfill code to an
extension.

## Testing Instructions (or ideally a Blueprint)

- ~~CI~~
- [unit tests currently
fail](https://systemsrequests.wordpress.com/2025/02/11/github-actions-runs-slowly-and-are-canceled-on-the-automattic-wordpress-playground-private-repository/)
so you should run tests locally
  - `nvm use 20` (to avoid crypto errors)
  - `npx nx run php-wasm-node:test`
## Motivation for the change, related issues
Details here:
https://playgroundp2.wordpress.com/2025/02/11/studio-php-execution-in-playground-cli-child-process/

## Implementation details
The PR adds the new `runCLI` function that exports the `Express` server
and the `PHPRequestHandler`.

The caller then can access to the `PHP` object as well as start/stop the
server by doing, for example:
```javascript
const res = runCLI( { command: 'server', ... } );

res.server.close( () => {
	res.requestHandler.getPrimaryPhp().exit();
} );
```

## Testing Instructions (or ideally a Blueprint)
Ensure this is still working with various flags passed:
```
bun ./packages/playground/cli/src/cli.ts server
```
Then build the package using `npm run build` and check if the
`dist/packages/playground/cli/index.js` file exists with a `runCLI`
exported.
## Motivation for the change, related issues

We are deploying a private Playground instance and want it to reference
itself rather than the original public site. In addition, these changes
will help folks who also want to self-host.

## Implementation details

In general, this PR replaces references to playground.wordpress.net with
relative URL references where those references might interfere with
Playground platform advances deployed to a private Playground instance.

There are a number of references to
`https://playground.wordpress.net/plugin-proxy.php`, and I am leaving
those alone for now. `plugin-proxy.php` requires a GITHUB_TOKEN
environment var, which requires further configuration, and this would be
particularly annoying for folks running local Playground servers. For
now, let's just keep using the public hosted version of that script and
adjust later if required.

In other cases, like that of the reference to
https://playground.wordpress.net/logger.php, I am leaving the references
as-is until there is a benefit to changing them.

## Testing Instructions (or ideally a Blueprint)

- CI
## Motivation for the change, related issues

Building php-wasm with WITH_SOURCEMAPS=yes is currently broken. When
WITH_SOURCEMAPS is enabled, it turns out Emscripten is generating JS
that uses single quotes rather than double-quotes. I guess this has to
do with the difference in optimization options but have not tested it.

## Implementation details

This PR fixes the build so there is no longer an error by expanding the
php-wasm JS replacement regexes to match on either single or double
quotes. I have not tested actually debugging with sourcemaps, but this
fixes the build error.

Once this is fixed I plan add a build arg for compiling php-wasm with
DWARF debug info so we [can step-through-debug the PHP runtime in
Chrome](https://developer.chrome.com/docs/devtools/wasm) with access to
variable values.

## Testing Instructions (or ideally a Blueprint)

Run the following command on trunk and on this branch, and note that it
fails on trunk and succeeds on this branch.
```
npx nx recompile-php:asyncify php-wasm-web -- --PHP_VERSION=8.4 --WITH_SOURCEMAPS=yes
```
@adamziel
Copy link
Collaborator Author

What's weird is that they pass in trunk

@brandonpayton
Copy link
Member

What's weird is that they pass in trunk

That is weird.

One thing is that we are bringing in different versions of Nx and Vitest from trunk. Vitest on trunk is "0.32.4", and in this PR, it is "2.1.9". It's possible the issue has something to do with that difference.

We could try upgrading to Vitest 3.x to see if the results differ. The failures could also be unrelated.

@brandonpayton brandonpayton force-pushed the merge-playground-private-2 branch from 9545344 to f495f97 Compare May 30, 2025 19:13
@brandonpayton
Copy link
Member

For now, I removed all my exploratory commits and left the two fixes intact.

If limiting PHP versions per test run seems to help, maybe we could further split up the unit tests by PHP version ranges. I don't love the idea of seeing 20+ unit test jobs in CI, but if it fixes the issue, that would be fine. :)

@adamziel
Copy link
Collaborator Author

Maybe related: vitest-dev/vitest#6285 (comment)

export default defineConfig({
    test: {
      pool: 'forks',
      poolOptions: {
        forks: {
          singleFork: true,
        },
      },
    },
  });

(Although I feel like we've tried that already?)

@brandonpayton
Copy link
Member

      singleFork: true,

...
(Although I feel like we've tried that already?)

I didn't try this specific option. From the vitest comment, it sounds like maybe it helped them track the error down. I added that setting, and we can see what the output shows.

@brandonpayton
Copy link
Member

I did the following but haven't seen anything promising or more revealing in the output:

  • poolOptions.forks.singleFork: true
  • Be explicit about vitest forks isolate flag, setting it to true. It is documented to default to "true", but we might as well test it.
  • Explicitly declare vitest uses a "forks" pool for php-wasm-node tests. This is also the documented default, but we can confirm.

I've also done the following to hopefully give us some interesting results but have to go AFK for the night before results are in:

  • Switched the php-wasm-node tests to use vmForks pool
  • Set fileParallelism to false for php-wasm-node tests

@brandonpayton
Copy link
Member

I've also done the following to hopefully give us some interesting results but have to go AFK for the night before results are in:

* Switched the php-wasm-node tests to use vmForks pool
* Set fileParallelism to false for php-wasm-node tests

I stuck around and saw tests continue failing after both of these changes.

I canceled the CI run for the most recent commit and restarted it with debug logging enabled. Hopefully we will learn something.

Some things we can consider:

  1. Finding a runner with more resources. This would be good if we could get it, but I think there is something going on here that would be good to understand better. We've seen problems like this before, and it hinders us to have to avoid the issue without understanding.
  2. Try updating to the latest Vitest. I'm trying this upgrade in a PR in the private Playground repo but so far am facing different test failures.

@mho22
Copy link
Collaborator

mho22 commented May 31, 2025

@brandonpayton There is a typo on line 200 in php-wasm/node/project.json :

- testphp-asyncify-gethostbyname
+ test-php-asyncify-gethostbyname

It's no surprise that single test is passing.

@brandonpayton
Copy link
Member

Nice find, @mho22!

Thank you for looking at this.
By the way, if you want to try any other ideas here, please feel free to remove or revert any of the troubleshooting commits.

@mho22
Copy link
Collaborator

mho22 commented May 31, 2025

@brandonpayton I believe I've found a potential solution, though it may not be ideal. Previously, in private, we had a custom runner capable of managing those tests. However, here, most tests crash when running the https version, while http passes. Therefore, we could consider duplicating the tests for both http and https. Additionally, we might need to add two new test files for php-request-handler.spec.ts.

In the Intl support PR, I had to separate them. So, instead of 7 test-unit-asyncify, we might end up with something like 14 test-unit-asyncify. I am currently testing this approach locally. Let's hope it works, and we can find a more elegant solution once it passes.

@mho22
Copy link
Collaborator

mho22 commented May 31, 2025

The setup is functional with 14 dedicated test-unit-asyncify jobs. The first job fails due to timeouts.

I duplicated the files for http and https protocols, but I think I found out a way to avoid that duplication.

@mho22
Copy link
Collaborator

mho22 commented Jun 2, 2025

I don't have the necessary permissions to work directly on this pull request, so I have created a duplicate here: #2226.

@adamziel
Copy link
Collaborator Author

adamziel commented Jun 3, 2025

Superseded by #2226

@adamziel adamziel closed this Jun 3, 2025
@github-project-automation github-project-automation bot moved this from Inbox to Done in Playground Board Jun 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.