test(php): add BDD suite wiring#3410
Open
countradooku wants to merge 5 commits into
Open
Conversation
|
Thanks for the PR. It is labeled Slash commands (own line, regular comment) move it around the queue:
See CONTRIBUTING.md for details. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3410 +/- ##
============================================
- Coverage 74.43% 74.40% -0.04%
Complexity 943 943
============================================
Files 1245 1245
Lines 121293 121293
Branches 97599 97628 +29
============================================
- Hits 90285 90242 -43
- Misses 28054 28059 +5
- Partials 2954 2992 +38
🚀 New features to boost your workflow:
|
Contributor
Author
|
/ready |
Add PHP to the shared BDD runner and component graph so PHP SDK scenario changes are detected and routed like the other foreign SDKs. The initial suite reuses the repository's shared basic messaging feature and runs it through PHPUnit with the existing PHP SDK test bootstrap. Constraint: PHP SDK requires PHP >=8.3, so the BDD image uses the trixie Rust base and the SDK composer.lock Rejected: Add Behat as a new BDD dependency | PHPUnit is already the PHP SDK test runner and can execute the shared scenario without expanding dependencies Confidence: high Scope-risk: narrow Directive: Keep bdd-php paths and sdk-php dependencies aligned when PHP SDK test tooling changes Tested: PHP syntax check for BasicMessagingFeatureTest.php Tested: PHPUnit XML parse Tested: docker compose config render for bdd/docker-compose.yml Tested: bdd-php component YAML dependency/task validation Tested: PHP BDD container against local Iggy server, OK (1 test, 38 assertions) Not-tested: Full ./scripts/run-bdd-tests.sh php on macOS because the shared BDD server image consumed local Mach-O target/debug binaries instead of Linux CI-built binaries
Keep PHP BDD files owned by the bdd-php component instead of the sdk-php component. The bdd-php component already depends on sdk-php, which gives BDD jobs the SDK prerequisites without running PHP SDK lint/build/test for BDD-only changes. Also stop copying foreign/php/composer.lock into the PHP BDD image because that file is not tracked in CI. Constraint: Component detection should route bdd/php changes to BDD tasks, not PHP SDK pre-merge tasks Rejected: Track composer.lock for the SDK | broader package-management decision outside this BDD wiring fix Confidence: high Scope-risk: narrow Tested: component ownership YAML validation Tested: PHP syntax check for BasicMessagingFeatureTest.php Tested: PHPUnit XML parse Tested: docker compose config render for bdd/docker-compose.yml Not-tested: Full Docker BDD run locally because Docker daemon is unavailable in this session
The PHP BDD image compiles cargo-php and then the PHP extension in separate Docker layers. CI ran out of disk during the extension build because the cargo install cache remained in the image filesystem before compiling the SDK. Constraint: GitHub Actions runners have limited Docker build disk for BDD images Rejected: Drop PHP BDD from the matrix | leaves issue apache#3310 only partially wired Rejected: Remove generated PHP extension install | the BDD suite needs the real SDK extension Confidence: medium Scope-risk: narrow Directive: Keep the BDD image cargo-php version aligned with the PHP SDK CI action Tested: git diff --check Tested: bash -n scripts/run-bdd-tests.sh Tested: php -l bdd/php/tests/BasicMessagingFeatureTest.php Tested: docker compose -f bdd/docker-compose.yml config Tested: docker compose -f bdd/docker-compose.yml build php-bdd Not-tested: Full ./scripts/run-bdd-tests.sh php after image build in this turn
The PHP test job was restoring the shared dev Rust cache, which had grown large enough to fill the GitHub runner during extraction before PHP tests could run. PHP builds only need their own extension artifacts, so use a PHP-specific cache namespace instead of the global workspace cache. Constraint: GitHub Actions PHP test runners had 28 GiB free after cleanup but the shared cache extraction still exhausted disk Rejected: Disable Rust setup entirely | PHP still needs the pinned Rust toolchain and system setup Confidence: medium Scope-risk: narrow Directive: Do not point PHP SDK jobs back at the shared dev cache without checking cache size on CI Tested: YAML parse for .github/actions/php/pre-merge/action.yml Tested: cargo fmt --manifest-path foreign/php/Cargo.toml -- --check Tested: cargo metadata --manifest-path foreign/php/Cargo.toml --format-version 1 --no-deps Tested: php composer.json JSON parse and php -l foreign/php PHP files Tested: git diff --check Not-tested: Full PHP CI job locally because the failure is specific to GitHub cache extraction
The PR now runs against the updated master license checker, which uses HawkEye instead of addlicense. Format the PHP BDD XML header with HawkEye and keep the strict PHP test file on its existing valid in-PHP Apache header by excluding it from HawkEye's generic PHP mapping. Constraint: Current HawkEye config maps generic PHP files to double-slash headers before <?php, which breaks strict PHP files Rejected: Keep HawkEye-generated double-slash header before <?php | emits output and breaks strict_types parsing Confidence: high Scope-risk: narrow Directive: Do not remove the BasicMessagingFeatureTest.php exclusion unless the global PHP HawkEye mapping is fixed for strict PHP files Tested: ./scripts/ci/license-headers.sh --check with hawkeye 6.5.1 Tested: php -l bdd/php/tests/BasicMessagingFeatureTest.php Tested: php SimpleXMLElement parse for bdd/php/phpunit.xml.dist Tested: bash -n scripts/run-bdd-tests.sh Tested: docker compose -f bdd/docker-compose.yml config Tested: ./scripts/ci/taplo.sh --check --ci licenserc.toml Tested: shellcheck scripts/ci/license-headers.sh Tested: git diff --check
countradooku
force-pushed
the
issue-3310-php-bdd
branch
from
55eff3b to
7889936
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
bdd/phpwith a PHPUnit implementation of the shared basic messaging BDD scenariophp-bddinto the shared Docker Compose BDD runner andscripts/run-bdd-tests.shbdd-phpcomponent detection and include PHP BDD paths insdk-phpCloses #3310.
Verification
php -l bdd/php/tests/BasicMessagingFeatureTest.phpbdd/php/phpunit.xml.distwith PHPSimpleXMLElementbash -n scripts/run-bdd-tests.shgit diff --checkdocker compose -f bdd/docker-compose.yml configbdd-phptask/dependenciesOK (1 test, 38 assertions)Notes
A full local
./scripts/run-bdd-tests.sh phprun on macOS failed before PHP tests because the shared BDD server image copied localtarget/debugMach-O binaries into a Linux container. CI builds Linux binaries before invoking the BDD runner, so the shared path should be valid there.