[aik099]

The WEB_FIXTURES_BROWSER and SELENIUM_VERSION were read via getenv(), so setting them as <server ...> in a local phpunit.xml had no effect (PHPUnit only writes $_SERVER for these). They worked only in CI, where they come as real env vars.

Changes:

• read both from $_SERVER (CLI copies env vars there, so CI still works)
• add commented-out examples to phpunit.xml.dist (they must stay commented-out, because an active <server ...> overwrites what CI passes via env)
• fix PHPStan level 10 complaints about mixed coming from $_SERVER

Closes #44

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.86%. Comparing base (ddce443) to head (59e9975).

Additional details and impacted files

@@            Coverage Diff            @@
##               main      #66   +/-   ##
=========================================
  Coverage     88.86%   88.86%           
  Complexity      197      197           
=========================================
  Files             1        1           
  Lines           494      494           
=========================================
  Hits            439      439           
  Misses           55       55           

Flag	Coverage Δ
chrome	88.25% <ø> (ø)
chromium	87.85% <ø> (ø)
edge	88.25% <ø> (ø)
firefox	88.05% <ø> (ø)
selenium-2.53.1	87.04% <ø> (ø)
selenium-3	88.05% <ø> (ø)
selenium-4	88.46% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[aik099]

The PHPStan check appears to be failed before my changes.

I've started a fresh build on main branch at https://github.com/minkphp/webdriver-classic-driver/actions/runs/37290349573 , and it confirms that failures are pre-existing (though needed to be fixed anyway).

[stof]

This will break cases that already have a local phpunit.xml using <env>

[aik099]

This will break cases that already have a local phpunit.xml using <env>

It will. However, developers should be creating their phpunit.xml based on provided phpunit.xml.dist file that don't have any of that.

If developers put whatever PHPUnit unit allows into config without understanding how it's consumed in the test suite itself, then it won't work either way.

The shipped phpunit.xml.dist is designed like this:

• the <server keys that are commented-out could be overridden from the CI side;
• once the phpunit.xml is created based on phpunit.xml.dist the developer uncomments needed <server ... keys and continue working, and for him they become like ENV for CI that always wins.

[stof]

The discussion in #44 makes me think we should rather document the usage of <env>. The argument that <env> does not override an actual env variable looks like a benefit rather than a drawback to me (as it means you can pass an actual env variable and have it used, instead of having to make sure to use a phpunit.xml file in which the <server> entry is commented)

[aik099]

The discussion in #44 makes me think we should rather document the usage of <env>. The argument that <env> does not override an actual env variable looks like a benefit rather than a drawback to me (as it means you can pass an actual env variable and have it used, instead of having to make sure to use a phpunit.xml file in which the <server> entry is commented)

@stof, I've added #44 (comment) comment showing more details in there. Please see there.

[uuf6429]

@aik099 mind merging back main into this and resolve conflicts, please?

[aik099]

@aik099 mind merging back main into this and resolve conflicts, please?

@uuf6429 , done. But please don't merge, because getEnvVar solution (on driver-testsuite-level) from the #44 (comment) is what we might end up using which is also a BC safe way.

[uuf6429]

@aik099 mind merging back main into this and resolve conflicts, please?

@uuf6429 , done. But please don't merge, because getEnvVar solution (on driver-testsuite-level) from the #44 (comment) is what we might end up using which is also a BC safe way.

OK, but marking the PR as draft helps.

[aik099]

Technically, it's not a draft and it works, but an alternative solution might arrive in different PR.

Marking as a draft to avoid any future confusion anyway.