-
Notifications
You must be signed in to change notification settings - Fork 4
ci(FFESUPPORT-934): test the widened dependency ranges via a matrix #67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
95adc3a
130d98e
b221eb7
d141fbc
66bd9cb
10ce3bf
f636a90
30f5aec
f821c51
57c8661
39c1910
f155c29
83f92ac
4b6a150
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| version: 2 | ||
|
|
||
| # Grouped: a week of transitive bumps arrives as one PR, not one per package. | ||
| updates: | ||
| - package-ecosystem: composer | ||
| directory: "/" | ||
| schedule: | ||
| interval: weekly | ||
| day: monday | ||
| open-pull-requests-limit: 5 | ||
| # This is a library. Widen the declared range, and keep the old versions | ||
| # that consumers still resolve. | ||
| versioning-strategy: widen | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 Claude working with Aaron: This file exists to make keeping The cost of a lock is churn. Grouping turns a week of transitive bumps into one pull request.
|
||
| # Let the ecosystem vet a release before we take it. Security updates ignore | ||
| # cooldown, so an advisory fix still arrives at once. | ||
| cooldown: | ||
| default-days: 7 | ||
| semver-patch-days: 5 | ||
| semver-minor-days: 14 | ||
| semver-major-days: 30 | ||
| groups: | ||
| # One PR for all open advisories, prod and dev. | ||
| composer-security: | ||
| applies-to: security-updates | ||
| patterns: ["*"] | ||
| # require-dev bumps reach nobody who installs the SDK. | ||
| composer-development: | ||
| applies-to: version-updates | ||
| dependency-type: development | ||
| patterns: ["*"] | ||
| # Majors stay ungrouped. They need a composer.json change and a release note. | ||
| composer-production: | ||
| applies-to: version-updates | ||
| dependency-type: production | ||
| patterns: ["*"] | ||
| update-types: ["minor", "patch"] | ||
|
|
||
| - package-ecosystem: github-actions | ||
| directory: "/" | ||
| schedule: | ||
| interval: weekly | ||
| day: monday | ||
| # The GitHub Actions ecosystem supports default-days only. | ||
| cooldown: | ||
| default-days: 7 | ||
| groups: | ||
| github-actions: | ||
| applies-to: version-updates | ||
| patterns: ["*"] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,10 @@ on: | |
|
|
||
| pull_request: | ||
|
|
||
| # Catches upstream releases that break our declared ranges. | ||
| schedule: | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 Claude working with Aaron: A weekly run catches an upstream release that breaks a declared range. The range is now wide, so a new Symfony release can break us without any commit here. The schedule finds it before a consumer does. |
||
| - cron: '17 6 * * 1' | ||
|
|
||
| workflow_dispatch: | ||
|
|
||
| workflow_call: | ||
|
|
@@ -28,36 +32,85 @@ permissions: | |
| contents: read | ||
|
|
||
| jobs: | ||
| build-and-test: | ||
| test-matrix: | ||
|
|
||
| runs-on: ubuntu-latest | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| include: | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 Claude working with Aaron: Six legs, one per reachable branch of the
|
||
| # The locked legs install what composer.lock pins. The lowest and | ||
| # highest legs resolve, and each Symfony major raises its PHP floor, | ||
| # so the runner PHP picks the branch of the symfony/cache constraint: | ||
| # 8.1 takes ^6.4, 8.3 takes ^7.0, 8.4 takes ^8.0. | ||
| - { php: '8.1', deps: locked } | ||
| - { php: '8.3', deps: locked } | ||
| - { php: '8.1', deps: lowest } | ||
| - { php: '8.1', deps: highest } | ||
| - { php: '8.3', deps: highest } | ||
| - { php: '8.4', deps: highest } | ||
|
aarsilv marked this conversation as resolved.
|
||
|
|
||
| name: PHP ${{ matrix.php }}, ${{ matrix.deps }} | ||
|
|
||
| steps: | ||
| - name: Display Testing Details | ||
| run: | | ||
| echo "Running SDK Test using" | ||
|
aarsilv marked this conversation as resolved.
|
||
| echo "Test Data: sdk-test-data@${TEST_DATA_BRANCH_NAME}" | ||
| echo "SDK Branch: php-sdk@${SDK_BRANCH_NAME}" | ||
| echo "PHP: ${{ matrix.php }}, dependencies: ${{ matrix.deps }}" | ||
|
|
||
| - uses: actions/checkout@v5 | ||
| with: | ||
| repository: ${{ github.event.pull_request.head.repo.full_name || 'Eppo-exp/php-sdk' }} | ||
| # A called workflow sees the caller's github context, so github.repository | ||
| # is the caller's repo. Name this repo to stay correct on both paths. | ||
| repository: ${{ github.repository == 'Eppo-exp/php-sdk' && github.event.pull_request.head.repo.full_name || 'Eppo-exp/php-sdk' }} | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 Context from Claude working with Aaron: This expression looks redundant, so here is why it is not. A workflow called with |
||
| ref: ${{ env.SDK_BRANCH_NAME }} | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| # No step needs git auth. make test clones test data from a public repo. | ||
|
aarsilv marked this conversation as resolved.
|
||
| persist-credentials: false | ||
|
|
||
| - name: Set up PHP | ||
| uses: shivammathur/setup-php@v2 | ||
| with: | ||
| php-version: ${{ matrix.php }} | ||
| extensions: json, pcntl, posix, sockets | ||
| coverage: none | ||
|
|
||
| # Must run before any resolution, or --strict cannot check lock sync. | ||
| - name: Validate composer.json and composer.lock | ||
| run: composer validate --strict | ||
|
|
||
| - name: Cache Composer packages | ||
| id: composer-cache | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: vendor | ||
| key: ${{ runner.os }}-php-${{ hashFiles('**/composer.lock') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-php- | ||
| # config.platform.php guards composer.lock. It must not cap a throwaway | ||
| # resolution, or symfony/cache ^8 stays unreachable. | ||
| - name: Target the runner's PHP for highest-version resolution | ||
| if: matrix.deps == 'highest' | ||
| run: composer config --unset platform.php | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 Claude working with Aaron: This step is what makes the
Nothing is committed back. The runner rewrites its own copy of the lock. |
||
|
|
||
| - name: Install dependencies | ||
| run: composer install --prefer-dist --no-progress | ||
| uses: ramsey/composer-install@4.0.0 | ||
| with: | ||
| dependency-versions: ${{ matrix.deps }} | ||
| composer-options: --prefer-dist | ||
|
|
||
| - name: Show resolved direct dependencies | ||
| run: composer show --direct | ||
|
|
||
| # --no-dev: a dev-only advisory reaches nobody who installs the SDK. | ||
| - name: Audit production dependencies | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 Claude working with Aaron: The audit runs on the
Known gap: the lock pins the 6.4 line, so an advisory against 7.x or 8.x is not covered. That is a follow-up decision about CI noise. |
||
| if: matrix.deps == 'locked' | ||
| run: composer audit --no-dev | ||
|
|
||
| - name: Run tests | ||
| run: make test branchName=${{env.TEST_DATA_BRANCH_NAME}} | ||
|
|
||
| # One stable check for every leg. Branch protection keeps the name | ||
| # build-and-test, whatever the matrix holds. | ||
| build-and-test: | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 Claude working with Aaron: This job keeps branch protection working, whatever the matrix holds. A matrix renames a job into one check per leg. Any required check named Verified with a deliberate failure: the leg failed, and this job failed with it. |
||
| if: always() | ||
| needs: test-matrix | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Check the matrix result | ||
| run: | | ||
| echo "Matrix result: ${{ needs.test-matrix.result }}" | ||
| [ "${{ needs.test-matrix.result }}" = "success" ] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,6 @@ name: Package Integration Testing | |
| on: | ||
| push: | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 Claude working with Aaron: This trigger started a package test for every new branch. The workflow declared A tag push honours the filter. Note the separate problem: the packaging test also fails on pushes to |
||
| branches: [ main ] | ||
| create: | ||
| tags: | ||
| - v* | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,4 +44,4 @@ test-data: | |
|
|
||
| .PHONY: test | ||
| test: test-data | ||
| ./vendor/phpunit/phpunit/phpunit tests | ||
| ./vendor/bin/phpunit tests | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,11 +26,10 @@ | |
| "ext-json": "*", | ||
| "psr/simple-cache": "3.*", | ||
| "psr/log": "^2.0|^3.0", | ||
| "shrikeh/teapot": "^2.3", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the reason/motivation to remove teapot?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe it wasn't being used but can double check!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 Claude working with Aaron: Correction to my reply above: teapot was in use. It supplied HTTP status constants at three import sites on
PR #64 inlined the literals. That is what created the magic numbers you flagged in the other thread. The removal still holds. The package was a production dependency that named integers and did nothing else. 66bd9cb replaces those integers with private constants on The tests keep literal status codes on purpose. A test that reads the same constant as the code cannot detect a wrong constant value.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 Context from Claude working with Aaron: One update to the reply above. The tests now name their statuses from the RFC again, via
|
||
| "composer/semver": "^3.4", | ||
| "php-http/discovery": "^1.17", | ||
| "webclient/ext-redirect": "^2.0", | ||
| "symfony/cache": "^6.4|^7.0" | ||
| "symfony/cache": "^6.4|^7.0|^8.0" | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 Claude working with Aaron: This line is the point of the PR, and it is @pkruithof's commit, unchanged. The branch keeps his widening and his GitHub builds the dependency graph from the lock, and that graph feeds our security alerts. FFESUPPORT-534, -734 and -887 all came from it. |
||
| }, | ||
| "require-dev": { | ||
| "phpunit/phpunit": "^9.6", | ||
|
|
@@ -39,7 +38,8 @@ | |
| "ext-pcntl": "*", | ||
| "psr-mock/http": "^1.0", | ||
| "squizlabs/php_codesniffer": "^3.10", | ||
| "ext-sockets": "*" | ||
| "ext-sockets": "*", | ||
| "teapot/status-code": "^2.1" | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 Context from Claude working with Aaron: This is the constants-only sibling of |
||
| }, | ||
| "config": { | ||
| "allow-plugins": { | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
recommend a
cooldownblock to allow some time for releases to be vetted in the ecosystem for exploits/bugs before opening a PRThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh good idea!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 Claude working with Aaron:
Added in 66bd9cb, with your values.
GitHub Actions supports
default-daysonly, so that ecosystem gets 7 days and no semver keys. Composer gets all four.Cooldown covers version updates only. A security advisory still opens a pull request at once.