Skip to content

ext/session: pass ini options to extra processes in tests #11294

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

Merged
merged 1 commit into from
May 24, 2023

Conversation

mikhainin
Copy link
Contributor

The change and the problem are similar to #11004.

When we build with --enable-session=shared, the tests are running with -d session.so but as long as we don't pass this argument to the inner process, it will fail with undefined function session_start().

@mikhainin
Copy link
Contributor Author

It seems like this broke the build on Windows... Is there a way to take a look at the failure?

@mikhainin
Copy link
Contributor Author

mikhainin commented May 23, 2023

Interestingly, the failure looks like this:

     *** Testing session_regenerate_id() : basic functionality for cookie ***
002- string(%d) "X-Powered-By: PHP/%d.%d.%s
002+ string(698) "
003+ Warning: PHP Startup: Unable to load dynamic library 'mysqli' (tried: C:\php\ext\mysqli (The specified module could not be found), C:\php\ext\php_mysqli.dll (The specified module could not be found)) in Unknown on line 0
004+ X-Powered-By: PHP/8.3.0-dev
     Expires: %s
     Cache-Control: no-store, no-cache, must-revalidate
     Pragma: no-cache
--

@mikhainin mikhainin marked this pull request as draft May 23, 2023 10:38
@mikhainin mikhainin force-pushed the test-extra-arguments-session branch from dd555d1 to f0a1aba Compare May 23, 2023 13:06
@mikhainin
Copy link
Contributor Author

I also removed -n argument, we need to somehow pass the extension directory. The script that runs the test gets this directory from ini-file and if the inner process doesn't read the config, it cannot find what we passed in the -d extension=...

@mikhainin mikhainin marked this pull request as ready for review May 23, 2023 14:19
@iluuu1994 iluuu1994 merged commit 2eee46e into php:master May 24, 2023
@iluuu1994
Copy link
Member

Great! Thanks @negram!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants