Fix bug71610.phpt - #16063
Fix bug71610.phpt#16063cmb69 wants to merge 1 commit into
Conversation
Apparently example.org now rejects POST requests, so we would need to
adjust the test expectation ("Method not allowed"). However, there is
no need for an online test; instead we're just using the CLI test
server. The serialization is a bit fiddly, but as long as there are
no quotes in `PHP_CLI_SERVER_ADDRESS` we're fine.
|
Nice, I never run online tests so I suppose that's why I haven't noticed this.
I'd say not only the tests :P |
ndossche
left a comment
There was a problem hiding this comment.
Pulled and tested locally, works fine. Thanks
| if (!file_exists(__DIR__ . "/../../../sapi/cli/tests/php_cli_server.inc")) { | ||
| echo "skip sapi/cli/tests/php_cli_server.inc required but not found"; | ||
| } |
There was a problem hiding this comment.
I think that we should assume this file to always exist, and let the test fail if it does not. Otherwise it will silently become always-skipped if we move the file without updating it.
I would suggest to use include __DIR__ . "/../../../sapi/cli/tests/skipif.inc" instead.
There was a problem hiding this comment.
Well, for our purposes, at least regarding CI, this is certainly a good idea. However, some may build ext/soap via phpize (I assume distro managers do that), and in that case it is unlikely that the file exists. include will trigger a warning, and the test would be reported as borked.
Given that the file is already used elsewhere, that opcache has its own (diverged) copy, and that I would like to use this server for ext/standard/tests/http and ext/ftp/tests (these currently have servers which require the posix extension), I hope we can come up with a more general solution.
Not perfect, and might have issues, but what about storing symlinks in the repository?
There was a problem hiding this comment.
I didn't realize these tests could be executed in a standalone pecl build. Skipping the test like this seems reasonable then.
I agree that it would be nice to have a more general solution at some point.
There was a problem hiding this comment.
Not perfect, and might have issues, but what about storing symlinks in the repository?
Do you mean symlinking the .inc file into ext/soap so that exports of the ext source code also contain it? I don't have experience with symlinks in git, but this could be an option indeed. An alternative could be to distribute/install a testing library along with run-tests.php or phpize (with the downside that evolving this library would be subject to BC).
There was a problem hiding this comment.
I've already did #16066, but @iluuu1994 suggested to go with a simple PHP include file instead of the symlinks (these have issues anyway; you need Windows 10 with developer mode enabled to have them properly work, and a somewhat recent git version, properly configure to accepts symlinks). And I guess, exporting the sources will not automatically resolve the symlinks. We could do that manually, though, through ./configure. (not sure about the details)
An alternative could be to distribute/install a testing library along with run-tests.php or phpize (with the downside that evolving this library would be subject to BC).
Yeah, that is what I would like, but I also see the problems.
Still, a first step to avoid those lengthy "parent paths" would be a good thing[TM].
|
macOS doesn't like me :( |
Huh, it seems like the testing step didn't even start? |
Yeah, that's possible. I've restarted the macOS run. |
Apparently example.org now rejects POST requests, so we would need to
adjust the test expectation ("Method not allowed"). However, there is
no need for an online test; instead we're just using the CLI test
server. The serialization is a bit fiddly, but as long as there are
no quotes in `PHP_CLI_SERVER_ADDRESS` we're fine.
Closes phpGH-16063.
Apparently example.org now rejects POST requests, so we would need to adjust the test expectation ("Method not allowed"). However, there is no need for an online test; instead we're just using the CLI test server. The serialization is a bit fiddly, but as long as there are no quotes in
PHP_CLI_SERVER_ADDRESSwe're fine.I've noticed this when testing the new releases (https://github.com/cmb69/php-ftw/actions/runs/11030040982/job/30633774744#step:5:98), while the test ran fine two weeks ago.
While fixing the test I've noticed that the soap test suite could need some TLC.
PS: note to merged: the test has been moved to the bugs/ subdirectory in PHP-8.4, so the paths to server.inc need to be adjusted when merging upwards.