Skip to content

fix(test): reset is_windows() mock state in CIUnitTestCase tearDown - #10554

Open
gr8man wants to merge 1 commit into
codeigniter4:developfrom
gr8man:fix/is-windows-test-pollution
Open

fix(test): reset is_windows() mock state in CIUnitTestCase tearDown#10554
gr8man wants to merge 1 commit into
codeigniter4:developfrom
gr8man:fix/is-windows-test-pollution

Conversation

@gr8man

@gr8man gr8man commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Description
When mocking the operating system via is_windows(true) or is_windows(false), the internal static variable $mocked retains the set boolean value until is_windows(null) is explicitly invoked with an argument (func_num_args() === 1).

In CommonFunctionsTest::testIsWindowsUsingMock(), line 866 previously called is_windows(); with zero arguments. Because zero arguments were passed, $mocked was not reset and remained false. This resulted in test pollution for any subsequent tests executed in the same PHPUnit process that relied on auto-detection of the OS platform (DIRECTORY_SEPARATOR === '\\').

This PR addresses the issue by:

  1. Updating CommonFunctionsTest::testIsWindowsUsingMock() to call is_windows(null) so that it explicitly tests resetting the mock.
  2. Adding resetIsWindowsMock() to CIUnitTestCase::$tearDownMethods to automatically reset is_windows(null) after each test run, preventing static state leaks across the test suite.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
  • Unit testing, with >80% coverage
  • User guide updated (Not applicable - test framework internal fix)
  • Conforms to style guide

@michalsn michalsn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a dedicated regression test verifying that CIUnitTestCase::tearDown() resets the is_windows() mock. The updated CommonFunctionsTest resets it explicitly, so it doesn't cover automatic cleanup.

Comment on lines +72 to +74
protected $tearDownMethods = [
'resetIsWindowsMock',
];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing test implementation could overwrite this. Instead, please just reset it directly at the end of the tearDown() method.

*/
protected function resetIsWindowsMock(): void
{
if (function_exists('is_windows')) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to check for this function at this point? I don't believe so.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants