Skip to content

THRIFT-5951: Improve PHP library unit test coverage#3432

Merged
sveneld merged 5 commits into
apache:masterfrom
sveneld:THRIFT-5951
May 2, 2026
Merged

THRIFT-5951: Improve PHP library unit test coverage#3432
sveneld merged 5 commits into
apache:masterfrom
sveneld:THRIFT-5951

Conversation

@sveneld
Copy link
Copy Markdown
Contributor

@sveneld sveneld commented Apr 30, 2026

Improves PHP unit test coverage from ~81.8% to ~86.8% statement coverage.

New tests:

  • TJSONProtocolTest: scalar roundtrip, struct, map, list, set, message, nested structures
  • BoundaryValuesTest: boundary values for Binary, Compact and JSON protocols
  • TransportErrorScenariosTest: transport error handling

Expanded tests:

  • TForkingServerTest: PHPMock for pcntl_fork/pcntl_waitpid, serve loop, fork failure
  • TExceptionTest: rich field roundtrip via unified dataProvider (scalar, map, list, set)
  • TBinarySerializerTest: dataProvider, accelerated C extension mocks
  • TSimpleJSONProtocolTest: refactored into dataProvider-driven methods
  • Factory tests: added getTransport and new instance per call tests

Refactoring:

  • Consolidated fixtures into Fixture/ directory with Test prefix naming
  • Renamed ProcessorSpy to TestProcessor

707 tests, 1528 assertions, 0 errors, 5 skipped.

  • Did you create an Apache Jira ticket? (Request account here, not required for trivial changes)
  • If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
  • Did you squash your changes to a single commit? (not required, but preferred)
  • Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
  • If your change does not involve any code, include [skip ci] anywhere in the commit message to free up build resources.

Client: php

- Add TJSONProtocol comprehensive tests (scalar roundtrip, struct, map, list, set, message, nested)
- Add BoundaryValuesTest for Binary, Compact and JSON protocols (integer, double, string, bool boundaries)
- Add TransportErrorScenariosTest for transport error handling
- Expand TForkingServer tests with PHPMock for pcntl_fork/pcntl_waitpid
- Expand TException tests with rich field roundtrip (scalar, map, list, set, empty containers)
- Expand TBinarySerializer tests with dataProvider and accelerated extension mocks
- Refactor TSimpleJSONProtocol write tests into dataProvider-driven methods
- Add missing factory tests (getTransport, new instance per call)
- Consolidate test fixtures into Fixture/ directory with Test prefix
- Rename ProcessorSpy to TestProcessor, add TestRichException and TestSerializerStruct fixtures

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mergeable mergeable Bot added the php label Apr 30, 2026
sveneld and others added 4 commits April 30, 2026 22:23
Client: php

- Replace PHP_FLOAT_MAX/MIN/EPSILON with numeric literals (unavailable in PHP 7.1)
- Use -9223372036854775807 instead of PHP_INT_MIN to avoid TCompactProtocol float-to-int overflow on PHP 8.5

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Client: php

- TCompactProtocol::writeI64() crashed on PHP 8.5 when encoding
  PHP_INT_MIN (-2^63) because negation overflows to float.
  Write the zigzag varint directly for this edge case.
- Restore PHP_INT_MIN in BoundaryValuesTest (was workaround)
- Add TODO for PHP_FLOAT_MAX/MIN/EPSILON literals (PHP 7.2+)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… 8.5

Client: php

- Add getAccessibleMethod() to ReflectionHelper trait with PHP version check
- Use it in TForkingServerTest instead of direct setAccessible() calls

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…on PHP 8.5

Client: php

- Add early return for PHP_INT_MIN in readI64For32BitArchitectureDataProvider
  to avoid -PHP_INT_MIN overflow (same pattern as TCompactProtocol fix)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@sveneld sveneld merged commit 83dcfe2 into apache:master May 2, 2026
88 of 89 checks passed
sveneld added a commit to sveneld/thrift that referenced this pull request May 2, 2026
THRIFT-5951: Improve PHP library unit test coverage

Client: php

- Add comprehensive protocol tests for TJSONProtocol, boundary values, transport errors, serializers and exceptions
- Expand TForkingServer tests with PHPMock and ReflectionHelper access helpers
- Add missing factory tests and consolidate reusable test fixtures under Fixture/
- Fix PHP_INT_MIN handling in TCompactProtocol::writeI64() on PHP 8.5 by encoding the zigzag varint edge case directly
- Avoid PHP_INT_MIN negation overflow in TBinaryProtocolTest data provider
- Document PHP_FLOAT_* literal compatibility TODOs

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant