Skip to content

Conversation

robberphex
Copy link
Contributor

BUG 43283 wasn't fixed
And this pr is for fixing https://bugs.php.net/bug.php?id=43283

@weltling
Copy link
Contributor

Thanks for the PR. The mentioned ticket was fixed in the documentation. Of course, one can still pursue to fix the behavior, in which case tests are required.

Thanks.

@robberphex
Copy link
Contributor Author

robberphex commented Jun 15, 2018

Could you tell me is there any way to test executing stdin?

I read run-test.php, it seems haven't this mode.

@weltling

@weltling
Copy link
Contributor

@robberphex i was not talking about run-tests.php. This patch is likely not use case for it. But inside a test, something like shell_exec(TEST_PHP_EXECUTABLE . " < some_script.php") could be done to test this patch.

Thanks.

@robberphex
Copy link
Contributor Author

@weltling Actually, shell_exec(TEST_PHP_EXECUTABLE . " < some_script.php") works.

But if we do this, the TEST_PHP_EXECUTABLE will load global php.ini, and doesn't load config from phpt file.

  1. the setting in phpt file, doesn't affect.
  2. the unittest depends on global php.ini, not php.ini generated by phpt.

So, I don't think use shell_exec(TEST_PHP_EXECUTABLE . " < some_script.php") is a good idea.
Waiting for your reply, @weltling.

@nikic
Copy link
Member

nikic commented Jul 16, 2018

I thought I added a separate environment variable that exports $php $pass_options $ini_settings, but I guess that was in some local branch only. I'd suggest modifying run-tests.php to add this, as it's also going to be useful for other things.

@nikic nikic closed this Jul 16, 2018
@nikic nikic reopened this Jul 16, 2018
@remicollet
Copy link
Member

@nikic isn't TEST_PHP_ARGS enough ?

@weltling
Copy link
Contributor

I've missed such options, especially for the cases where extensions like openssl or curl are tested with the integrated HTTP server. But, sometimes it is still not enough - for example when compiling with a non standard library, on would still need to adjust the environment, too. Or, when extensions are shared.

For this particular case, it should be actually enough to pass -n. It only needs to check whether the constants are defined by the core.

Thanks.

@krakjoe
Copy link
Member

krakjoe commented Oct 2, 2019

Since the original ticket is marked closed, and since there is no movement here in 18 months, I'm closing this.

If there is to be more work on this issue, please reference this PR from the new request.

@krakjoe krakjoe closed this Oct 2, 2019
@nikic
Copy link
Member

nikic commented Oct 2, 2019

Looks like this got stuck on testing ... a bit late, but I think in this particular case the answer is to just pass -n. There's no need for loading an ini file here.

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.

5 participants