test(php): add BDD suite wiring#3410
Open
countradooku wants to merge 5 commits into
Open
Conversation
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
|
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.42% 74.41% -0.02%
Complexity 943 943
============================================
Files 1231 1231
Lines 120911 120911
Branches 97645 97676 +31
============================================
- Hits 89994 89979 -15
+ Misses 27958 27943 -15
- Partials 2959 2989 +30
🚀 New features to boost your workflow:
|
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 lint task should validate source formatting and PHP syntax without loading the compiled extension outside the PHP runtime. The existing cargo-php stub generation path dlopens the extension directly, which fails in CI because Zend symbols are provided by PHP when the extension is loaded normally. Constraint: cargo-php stubs dlopens libiggy_php.so without PHP exporting Zend symbols Rejected: Keep cargo-php stub regeneration in lint | it fails before syntax linting with undefined Zend symbols Confidence: medium Scope-risk: narrow Directive: Reintroduce generated-stub validation only through a path that loads the extension via the target PHP runtime Tested: YAML parse for PHP action and components config Tested: php -r composer.json JSON parse Tested: cargo fmt --manifest-path foreign/php/Cargo.toml -- --check Tested: php -l bdd/php/tests/BasicMessagingFeatureTest.php and foreign/php/iggy-php.stubs.php Tested: bash -n scripts/run-bdd-tests.sh Tested: docker compose -f bdd/docker-compose.yml config Not-tested: Full GitHub PHP lint job locally because the failure requires the Linux CI PHP extension-loading environment
The PHP lint job should keep validating generated stubs, but the build inputs must be stable so unrelated PRs do not fail when cargo-php or ext-php-rs floats to a version that cannot load the freshly built extension in CI. Constraint: PR apache#3411 already identified the stable cargo-php and ext-php-rs versions Rejected: Remove stub regeneration from lint | weaker than the existing CI contract and unnecessary once the toolchain is pinned Confidence: medium Scope-risk: narrow Directive: Keep cargo-php and ext-php-rs pins aligned with PHP stub generation behavior Tested: YAML parse for PHP action and components config Tested: php composer.json JSON parse and php -l foreign/php PHP files 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: bash -n scripts/run-bdd-tests.sh Tested: docker compose -f bdd/docker-compose.yml config Not-tested: Full PHP lint job locally because it requires the Linux CI PHP extension environment
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.