test: Add unit tests for Session FileHandler

[gr8man]

Description
Adds unit tests for system/Session/Handlers/FileHandler.php, reaching 100%
statement and method coverage. No production code changed.

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
• Conforms to style guide

[Copilot]

Pull request overview

Adds unit tests and failure-injection helpers for the Session FileHandler.

Changes:

• Covers constructor, file operations, cleanup, and failure paths.
• Adds configurable stream and close-failure test doubles.
• Assumed develop base; focused tests were not run.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File	Description
tests/system/Session/Handlers/FileHandlerTest.php	Adds comprehensive handler tests.
tests/_support/Session/FileHandlerCloseFail.php	Simulates failed session closing.
tests/_support/Session/FaultyStream.php	Simulates stream operation failures.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

tests/_support/Session/FileHandlerCloseFail.php:28

• Returning false without performing the parent's cleanup leaves the session file open and locked. The corresponding destroy test then relies on POSIX's ability to unlink an open file; on Windows destroy() returns false because the open file cannot be deleted. Close the resource normally, then report failure so the fallback branch remains exercised portably.

        return false;

tests/system/Session/Handlers/FileHandlerTest.php:242

• The OS requirement does not exclude a root test process. Under root, a 0555 directory is still writable, so open() succeeds and this expected-exception test fails (a common case in containers). Skip when the permission change cannot make the directory non-writable, or use a deterministic filesystem double.

        chmod($dir, 0555);

tests/system/Session/Handlers/FileHandlerTest.php:534

• A root process can still open a 0000 directory, so on root-based containers gc() scans this empty directory and returns 0 rather than false. The Linux/macOS attribute does not guard that case; verify that opendir() actually fails and skip otherwise, or use a deterministic failure double.

        chmod($dir, 0000);

[michalsn]

Thank you for adding tests for FileHandler. More coverage here is valuable, but I don't think reaching 100% should be the goal.

Several tests inspect or modify protected state through getPrivateProperty() and setPrivateProperty(). This makes internal implementation details part of the effective test contract and may cause harmless refactoring to break the suite.

Please refocus the tests on observable behavior: return values, exceptions, logs, created files, session isolation, and cleanup. Tests and fixtures that exist only to reach otherwise inaccessible branches can be removed unless they protect a meaningful regression or documented behavior.

It is completely acceptable for coverage to fall below 100%. A smaller suite for testing the stable public contract will be more valuable and easier to maintain.