Skip to content

fix: remove destructive setServerArray([]) in FiltersTest::setUp to prevent random-order test failures#10425

Merged
michalsn merged 2 commits into
codeigniter4:developfrom
gr8man:fix/filters-test-superglobals-pollution
Jul 22, 2026
Merged

fix: remove destructive setServerArray([]) in FiltersTest::setUp to prevent random-order test failures#10425
michalsn merged 2 commits into
codeigniter4:developfrom
gr8man:fix/filters-test-superglobals-pollution

Conversation

@gr8man

@gr8man gr8man commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Description

FiltersTest::setUp() called service('superglobals')->setServerArray([]) which sets $_SERVER = [] for the entire process. Immediately after, Services::injectMock('superglobals', new Superglobals()) creates a new Superglobals instance from the now-empty $_SERVER, resulting in a mock with no argv key.

When tests run in random order, this mock persists from FiltersTest to other test classes that lack their own setUp() service reset (e.g. CorsTest). When CorsTest::testBeforeDoesNothingWhenCliRequest creates new CLIRequest(), the constructor calls parseCommand()getServer('argv') which returns null from the empty mock, causing array_shift(null) to throw a TypeError.

The setServerArray([]) call is redundant — new Superglobals() without arguments already captures a clean snapshot of the current $_SERVER. Removing it restores the normal CLI argv values for all subsequent tests.

Verified with multiple random seeds, including the originally failing seed 1784667021.
https://github.com/codeigniter4/CodeIgniter4/actions/runs/29867283088/job/88758550319?pr=10417

@github-actions github-actions Bot added the testing Pull requests that changes tests only label Jul 21, 2026

@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.

LGTM!

@michalsn
michalsn merged commit aa7f6bb into codeigniter4:develop Jul 22, 2026
55 checks passed
@michalsn

Copy link
Copy Markdown
Member

Thank you @gr8man!

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

Labels

testing Pull requests that changes tests only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants