Skip to content

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

Merged
paulbalandan merged 1 commit into
codeigniter4:developfrom
gr8man:fix/is-windows-test-pollution
Sep 18, 2026
Merged

paulbalandan merged 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 thread system/Test/CIUnitTestCase.php Outdated
Comment thread system/Test/CIUnitTestCase.php Outdated
@gr8man
gr8man force-pushed the fix/is-windows-test-pollution branch from 0b294b5 to 8610c9c Compare September 13, 2026 20:04
@carson-codeigniter4 carson-codeigniter4 Bot added the bug Verified issues on the current code behavior or pull requests that will fix them label Sep 13, 2026
@michalsn

Copy link
Copy Markdown
Member

Looks good. Could you please rebase to fix the PHPStan failure?

@gr8man
gr8man force-pushed the fix/is-windows-test-pollution branch from 8610c9c to 5f10252 Compare September 14, 2026 19:11
@paulbalandan paulbalandan changed the title fix(test): reset is_windows() mock state in CIUnitTestCase tearDown fix(test): reset is_windows() mock state in CIUnitTestCase::tearDown() Sep 18, 2026
@paulbalandan paulbalandan changed the title fix(test): reset is_windows() mock state in CIUnitTestCase::tearDown() fix(Test): reset is_windows() mock state in CIUnitTestCase::tearDown() Sep 18, 2026
@paulbalandan
paulbalandan merged commit edf4380 into codeigniter4:develop Sep 18, 2026
59 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Verified issues on the current code behavior or pull requests that will fix them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants