Unit tests: Restore site-data.spec.ts

[adamziel]

The test was removed in #1192 due to intermittent failures like these:

FAIL  src/lib/steps/site-data.spec.ts > Blueprint step setSiteOptions() > should set the site option

They seem related to #1189, but:

• I can't reproduce it locally
• The test actually passed in Avoid partial munmap memory leak #1189 and only started failing in Blueprints: Rename importFile to importWxr, switch to humanmade/WordPress importer #1192
• All the other tests pass and the Playground website works so I wonder what's going on there.
• The stack trace mentions network activity, but that call shouldn't involve any. It's almost as if the trace was coming from php-networking.spec.ts, but those tests actually pass.

Is the CI runner just running out of memory? But then it shouldn't be able to allocate the memory segment in the first place. Weird!

CC @brandonpayton for insights

[adamziel]

I set the PHP version used in that test to 7.4 instead of 8.0 and now the tests pass. How weird!

[adamziel]

I set the PHP version used in that test to 8.0 again and they started failing in the same way.

[adamziel]

It doesn't even fail when setting the option, but when displaying it:

const response = await php.run({
	code: `<?php
                require '/wordpress/wp-load.php';
                echo get_option('blogname');
	`,
});

I wonder if some code path in there triggers a REST API request to api.wordpress.org. If yes, it could be an ASYNCIFY-related problem in disguise? I'm not sure what ZEND_SEND_ARRAY_SPEC_HANDLER is, as its error messages refer to both call_user_func_array() and array_slice(), but perhaps we need it in the ASYNCIFY functions list?

[adamziel]

Adding ZEND_SEND_ARRAY_SPEC_HANDLER to ASYNCIFY_ONLY list didn't help. Perhaps that's a legitimate error, after all? But why wouldn't it fail locally?

[adamziel]

Also the E2E failure is interesting:

  1 failing
  1) Playground website UI
       PHP extensions bundle
         should not load additional PHP extensions when not requested:
     Error: The following error originated from your application code, not from Cypress. It was caused by an unhandled promise rejection.
  > Error when executing the blueprint step #0 ({"step":"login","username":"admin","password":"password"}) : Aborted(). Build with -sASSERTIONS for more info.
When Cypress detects uncaught errors originating from your application it will automatically fail the current test.
This behavior is configurable, and you can choose to turn this off by listening to the `uncaught:exception` event.
https://on.cypress.io/uncaught-exception-from-application
      at Object.run (http://localhost/assets/config-20ddff8d.js:623:14922)
      at async runBlueprintSteps (http://localhost/assets/config-20ddff8d.js:623:16785)
      at async startPlaygroundWeb (http://localhost/assets/config-20ddff8d.js:627:9538)

Here's the screenshot:

[adamziel]

Interestingly, no related error reports came in on #playground-logs yet. I wonder if we're just hitting CI resource limits somehow, at least with respect to the E2E tests.

[adamziel]

Aha, I reverted PHP wasm to its version from before the memory leak PR and the tests passed. I'm going to revert #1189 until we can identify and resolve the crash.

[brandonpayton]

So far, I have not had any luck reproducing this locally.

In the absence of a better way, I wonder if it is possible to run a test branch in CI with more error-logging to help narrow things down.

[brandonpayton]

In the absence of a better way, I wonder if it is possible to run a test branch in CI with more error-logging to help narrow things down.

Just looked at the GH Actions tab, and it looks like running CI changes is exactly what happened for this PR. I'm planning to create a test PR for the memory leak fix and see if I can repro via CI at least.

[adamziel]

Reverting this branch to 096a017 would in theory get CI to fail again.