Usage of $_SERVER vs $_ENV

[aik099]

Current state:

• using <server ... in phpunit.xml.dist file to define default settings for test suite
• using $_SERVER in test suite to use the above-defined settings

Problem:

Inability to override the above settings during GitHub Actions builds because we can set environment variables (readable through $_ENV) and not server variables (readable through $_SERVER).

Solution

Do so Find & Replace in the phpunit.xml.dist on GitHub Actions instead of providing environment variables like DRIVER_URL and such, that don't override what phpunit.xml.dist defines.

[uuf6429]

Firstly, we shouldn't use $_ENV or $_SERVER, we should use getenv() instead.

Secondly, I'm sorry but I don't understand the solution - what do you want to replace? The linked PR doesn't make a lot of sense to me, since the browser will almost always by different.

Additionally, it's not clear to me why the PHPUnit XML should use <server> or <env> - that I cannot judge. My feeling is that using <env> allows overriding by cli env vars whereas <server> doesn't allow overriding?

[aik099]

Firstly, we shouldn't use $_ENV or $_SERVER, we should use getenv() instead.

I also like getenv better (not sure though how it is different from $_ENV). However, it will introduce a BC break for other drivers, because we need to replace $_SERVER usages in the driver-testsuite repo as well.

Secondly, I'm sorry but I don't understand the solution - what do you want to replace? The linked PR doesn't make a lot of sense to me, since the browser will almost always by different.

Replace <server name="WEB_FIXTURES_HOST" value="http://host.docker.internal:8002"/> with a host needed by a GitHub Actions.

Additionally, it's not clear to me why the PHPUnit XML should use <server> or <env> - that I cannot judge. ...

Because this is the only way to configure how test suite runs (e.g. what browser to use for testing, how to map paths inside Document Root and such stuff).

... My feeling is that using <env> allows overriding by cli env vars whereas <server> doesn't allow overriding?

According to PHPUnit documentation using <env.... will change $_ENV (and getenv result) unless such environment variable is provided externally.

Example 1:

1. I have <env name="WEB_FIXTURES_HOST" value="http://host.docker.internal:8002"/> in the PHPUnit config file
2. I run PHPUnit as vendor/bin/phpunit
3. the getenv / $_ENV would have WEB_FIXTURES_HOST key as http://host.docker.internal:8002

Example 2:

1. I have <env name="WEB_FIXTURES_HOST" value="http://host.docker.internal:8002"/> in the PHPUnit config file
2. I run PHPUnit as WEB_FIXTURES_HOST="my-custom-host:80" vendor/bin/phpunit
3. the getenv / $_ENV would have WEB_FIXTURES_HOST key as my-custom-host:80

If I had used <server instead of <env such a trick wouldn't have worked.

[uuf6429]

And we want example 2 to work, right? Then the phpunit xml should be using env, not server?

I also like getenv better (not sure though how it is different from $_ENV).

$_ENV is always case sensitive (because of normal PHP array access), but getenv is case-insensitive depending on OS. e.g. $_ENV['PATH'] breaks on Windows, but getenv('PATH') is more cross-platform.

getenv is also more convenient, because you don't have to use isset/empty($_ENV).

getenv is theoretically slower (because of function call), but the benefits are worth it.

[aik099]

And we want example 2 to work, right? Then the phpunit xml should be using env, not server?

Yes, but to make it work all usages of $_SERVER in the test suite (not only driver code) must be replaced at least with $_ENV.

As I said above that would be a BC break for all other drivers using the same test suite.

[stof]

env variables are readable through $_SERVER in PHP. We are already relying on that in other drivers when needed: https://github.com/minkphp/MinkSelenium2Driver/blob/db44be5055cb1dfcd323c7b375ff7cf97c187de9/.github/workflows/tests.yml#L87
And we do it also in the current driver:

webdriver-classic-driver/.github/workflows/ci.yml

Lines 118 to 122 in 61b12d0

	SELENIUM_VERSION: ${{ matrix.selenium }}
	DRIVER_URL: http://localhost:4444/wd/hub
	WEB_FIXTURES_HOST: http://host.docker.internal:8002
	WEB_FIXTURES_BROWSER: ${{ matrix.browser }}
	DRIVER_MACHINE_BASE_PATH: /fixtures/

Note also that getenv is not thread safe. This is not an issue for usages in the testsuite though. but it might be an issue in driver code (as we don't control how it is used)

[aik099]

env variables are readable through $_SERVER in PHP. We are already relying on that in other drivers when needed: https://github.com/minkphp/MinkSelenium2Driver/blob/db44be5055cb1dfcd323c7b375ff7cf97c187de9/.github/workflows/tests.yml#L87

This works only, when you don't have corresponding ENV var defined in PHPUnit config file:

webdriver-classic-driver/phpunit.xml.dist

Line 23 in 61b12d0

<server name="WEB_FIXTURES_HOST" value="http://host.docker.internal:8002"/>

This way if we define default env var values in PHPUnit config file they can't overridden via the environment given to PHPUnit runner itself.