Skip to content

Test Symfony 7.3 beta#18053

Merged
mpysiak merged 5 commits intoSylius:2.1from
GSadee:SYL-4858-sf-7.3
May 23, 2025
Merged

Test Symfony 7.3 beta#18053
mpysiak merged 5 commits intoSylius:2.1from
GSadee:SYL-4858-sf-7.3

Conversation

@GSadee
Copy link
Copy Markdown
Member

@GSadee GSadee commented May 21, 2025

Q A
Branch? 2.1
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Related tickets
License MIT

Summary by CodeRabbit

  • Bug Fixes

    • Improved validation for payment request handling to ensure hashes are not empty.
    • Corrected a configuration option in channel price history validation to align with expected data types.
  • Chores

    • Updated end-to-end testing workflows for enhanced stability and clearer logging.
    • Adjusted Composer settings to prefer stable packages in certain pipeline conditions.
    • Removed an unused development dependency from the customer component.

@GSadee GSadee added the Dependencies Pull requests that update a dependency file label May 21, 2025
@probot-autolabeler probot-autolabeler bot added the Maintenance CI configurations, READMEs, releases, etc. label May 21, 2025
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented May 21, 2025

Walkthrough

This update introduces two new jobs to the GitHub Actions unstable end-to-end workflow, adjusts Composer's prefer-stable setting for unstable pipelines, tightens hash validation in a payment response provider, corrects a validation constraint option in an XML file, and removes an unused Composer development dependency.

Changes

File(s) Change Summary
.github/workflows/ci_e2e-unstable.yaml Added behat-ui-js-chromedriver-unstable and behat-ui-js-panther-unstable jobs with specific matrices, environment setup, caching, Composer configuration, build steps, and artifact upload on failure.
RoboFile.php Changed Composer's prefer-stable setting to true when UNSTABLE is YES in the package pipeline process.
src/Sylius/Bundle/CoreBundle/OrderPay/Provider/PaymentRequestAfterPayResponseProvider.php Updated the supports method to check that the hash is neither null nor an empty string, improving validation logic.
src/Sylius/Bundle/CoreBundle/Resources/config/validation/ChannelPriceHistoryConfig.xml Fixed the Type constraint option from value to type for the lowestPriceForDiscountedProductsCheckingPeriod property.
src/Sylius/Component/Customer/composer.json Removed the "icanhazstring/composer-unused" package from the require-dev section.

Sequence Diagram(s)

sequenceDiagram
    participant GitHub Actions
    participant BuildTestAppAction
    participant Composer
    participant Behat
    participant ArtifactUploader

    GitHub Actions->>GitHub Actions: Start behat-ui-js-chromedriver-unstable job
    GitHub Actions->>Composer: Restore dependencies from cache
    GitHub Actions->>Composer: Configure prefer-stable true (if UNSTABLE)
    GitHub Actions->>BuildTestAppAction: Build application with e2e/js parameters
    GitHub Actions->>Behat: Run Chromedriver-tagged tests (with retries)
    Behat-->>GitHub Actions: Test results
    GitHub Actions->>Behat: Run @failing scenarios (continue on error)
    Behat-->>GitHub Actions: Failing test results
    alt On failure
        GitHub Actions->>ArtifactUploader: Upload logs from etc/build/ and var/log
    end
Loading

Possibly related PRs

  • Sylius/Sylius#17846: Both PRs modify the .github/workflows/ci_e2e-unstable.yaml file to update or add jobs related to the unstable CI workflow, with the main PR adding new jobs using Symfony ^7.3@beta and the retrieved PR updating the Symfony version to 7.3.x-dev in the existing unstable workflow matrix, showing a direct connection in CI configuration changes targeting Symfony 7.3 versions.

Suggested labels

CI

Suggested reviewers

  • Rafikooo

Poem

In the warren where code hops free,
New jobs now run for all to see.
Composer prefers the stable way,
While hashes check their truth today.
Unused packages, gone with grace—
CI rabbits quicken the pace!
🐇✨

Note

⚡️ AI Code Reviews for VS Code, Cursor, Windsurf

CodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback.
Learn more here.

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions
Copy link
Copy Markdown

github-actions bot commented May 21, 2025

❗ Preview Environment delete from Bunnyshell failed

Check https://github.com/Sylius/Sylius/actions/runs/15205149026 for details.

See: Environment Details

Available commands:

  • 🚀 /bns:deploy to redeploy the environment
  • /bns:delete to remove the environment

mpysiak
mpysiak previously approved these changes May 23, 2025
@GSadee GSadee force-pushed the SYL-4858-sf-7.3 branch from c8db15c to f40d76a Compare May 23, 2025 06:23
@GSadee GSadee marked this pull request as ready for review May 23, 2025 06:50
@GSadee GSadee requested review from a team as code owners May 23, 2025 06:50
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🔭 Outside diff range comments (1)
.github/workflows/ci_e2e-unstable.yaml (1)

3-11: ⚠️ Potential issue

Define branch input under both triggers
The workflow references inputs.branch in several jobs, but neither workflow_call nor workflow_dispatch declares a branch input. This will lead to undefined values at runtime. Please add a branch input with a default (e.g., empty string) under both triggers:

on:
  workflow_call:
    inputs:
      ignore-failure:
        description: "Don't fail on error"
        required: false
        type: boolean
        default: false
      branch:
        description: "Git branch to checkout"
        required: false
        type: string
        default: ""
  workflow_dispatch:
    inputs:
      branch:
        description: "Git branch to checkout"
        required: false
        default: ""
🧹 Nitpick comments (1)
.github/workflows/ci_e2e-unstable.yaml (1)

91-245: Consider DRY’ing duplicated job definitions
The two JS jobs (chromedriver and panther) share nearly identical steps. To reduce maintenance overhead, you could extract common steps into YAML anchors or a reusable composite action, parameterizing only the differences (e.g., e2e_js, chrome_version).

# Example using an anchor for shared steps
.shared-steps: &shared-build
  - name: Get Composer cache directory
    id: composer-cache
    run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT
  - name: Restore dependencies
    uses: actions/cache@v4
    with:
      path: ${{ steps.composer-cache.outputs.dir }}
      key: ...
  - name: Change minimum-stability to dev
    run: |
      composer config minimum-stability dev
      composer config prefer-stable true
  - name: Build application
    uses: SyliusLabs/BuildTestAppAction@v2.4
    with:
      # shared parameters...

Then reference <<: *shared-build in each job before job-specific steps.

🧰 Tools
🪛 actionlint (1.7.7)

93-93: property "env" is not defined in object type {mysql: number; php: number; symfony: string}

(expression)


105-105: property "env" is not defined in object type {mysql: number; php: number; symfony: string}

(expression)


110-110: property "branch" is not defined in object type {ignore-failure: bool}

(expression)


113-113: property "branch" is not defined in object type {ignore-failure: bool}

(expression)


116-116: property "branch" is not defined in object type {ignore-failure: bool}

(expression)


175-175: property "env" is not defined in object type {mysql: number; php: number; symfony: string}

(expression)


187-187: property "env" is not defined in object type {mysql: number; php: number; symfony: string}

(expression)


192-192: property "branch" is not defined in object type {ignore-failure: bool}

(expression)


195-195: property "branch" is not defined in object type {ignore-failure: bool}

(expression)


198-198: property "branch" is not defined in object type {ignore-failure: bool}

(expression)

🪛 YAMLlint (1.37.1)

[warning] 109-109: too many spaces after hyphen

(hyphens)


[warning] 115-115: too many spaces after hyphen

(hyphens)


[warning] 119-119: too many spaces after hyphen

(hyphens)


[warning] 123-123: too many spaces after hyphen

(hyphens)


[warning] 130-130: too many spaces after hyphen

(hyphens)


[warning] 135-135: too many spaces after hyphen

(hyphens)


[warning] 149-149: too many spaces after hyphen

(hyphens)


[warning] 155-155: too many spaces after hyphen

(hyphens)


[warning] 162-162: too many spaces after hyphen

(hyphens)


[warning] 191-191: too many spaces after hyphen

(hyphens)


[warning] 197-197: too many spaces after hyphen

(hyphens)


[warning] 201-201: too many spaces after hyphen

(hyphens)


[warning] 205-205: too many spaces after hyphen

(hyphens)


[warning] 212-212: too many spaces after hyphen

(hyphens)


[warning] 217-217: too many spaces after hyphen

(hyphens)


[warning] 232-232: too many spaces after hyphen

(hyphens)


[warning] 235-235: too many spaces after hyphen

(hyphens)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8b7d6d7 and f40d76a.

📒 Files selected for processing (5)
  • .github/workflows/ci_e2e-unstable.yaml (1 hunks)
  • RoboFile.php (1 hunks)
  • src/Sylius/Bundle/CoreBundle/OrderPay/Provider/PaymentRequestAfterPayResponseProvider.php (1 hunks)
  • src/Sylius/Bundle/CoreBundle/Resources/config/validation/ChannelPriceHistoryConfig.xml (1 hunks)
  • src/Sylius/Component/Customer/composer.json (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/ci_e2e-unstable.yaml

93-93: property "env" is not defined in object type {mysql: number; php: number; symfony: string}

(expression)


105-105: property "env" is not defined in object type {mysql: number; php: number; symfony: string}

(expression)


110-110: property "branch" is not defined in object type {ignore-failure: bool}

(expression)


113-113: property "branch" is not defined in object type {ignore-failure: bool}

(expression)


116-116: property "branch" is not defined in object type {ignore-failure: bool}

(expression)


175-175: property "env" is not defined in object type {mysql: number; php: number; symfony: string}

(expression)


187-187: property "env" is not defined in object type {mysql: number; php: number; symfony: string}

(expression)


192-192: property "branch" is not defined in object type {ignore-failure: bool}

(expression)


195-195: property "branch" is not defined in object type {ignore-failure: bool}

(expression)


198-198: property "branch" is not defined in object type {ignore-failure: bool}

(expression)

🪛 YAMLlint (1.37.1)
.github/workflows/ci_e2e-unstable.yaml

[warning] 109-109: too many spaces after hyphen

(hyphens)


[warning] 115-115: too many spaces after hyphen

(hyphens)


[warning] 119-119: too many spaces after hyphen

(hyphens)


[warning] 123-123: too many spaces after hyphen

(hyphens)


[warning] 130-130: too many spaces after hyphen

(hyphens)


[warning] 135-135: too many spaces after hyphen

(hyphens)


[warning] 149-149: too many spaces after hyphen

(hyphens)


[warning] 155-155: too many spaces after hyphen

(hyphens)


[warning] 162-162: too many spaces after hyphen

(hyphens)


[warning] 191-191: too many spaces after hyphen

(hyphens)


[warning] 197-197: too many spaces after hyphen

(hyphens)


[warning] 201-201: too many spaces after hyphen

(hyphens)


[warning] 205-205: too many spaces after hyphen

(hyphens)


[warning] 212-212: too many spaces after hyphen

(hyphens)


[warning] 217-217: too many spaces after hyphen

(hyphens)


[warning] 232-232: too many spaces after hyphen

(hyphens)


[warning] 235-235: too many spaces after hyphen

(hyphens)

🔇 Additional comments (6)
src/Sylius/Component/Customer/composer.json (1)

39-39: LGTM! Clean dependency removal.

The removal of the unused icanhazstring/composer-unused development dependency is a good cleanup that aligns with the PR's focus on dependency management for Symfony 7.3 beta testing.

src/Sylius/Bundle/CoreBundle/Resources/config/validation/ChannelPriceHistoryConfig.xml (1)

26-26: Correct fix for validation constraint syntax.

The change from <option name="value">int</option> to <option name="type">int</option> is correct. The Type constraint uses the type option to specify the expected data type, not value. This fixes a validation configuration bug.

RoboFile.php (1)

66-66: Correct alignment with modern Composer practices for unstable testing.

The change from prefer-stable false to prefer-stable true when UNSTABLE=yes is correct. This follows the pattern of setting minimum-stability dev with prefer-stable true, which allows testing against development packages while still preferring stable versions when available.

src/Sylius/Bundle/CoreBundle/OrderPay/Provider/PaymentRequestAfterPayResponseProvider.php (1)

71-73: Enhanced input validation for payment request hash.

The strengthened validation in the supports() method now properly checks that the hash is both not null and not empty. This prevents potential issues with empty hash values and aligns well with the Assert::notNull validation in the getResponse() method.

.github/workflows/ci_e2e-unstable.yaml (2)

39-43: Composer config for unstable dependencies is correctly set
Great addition of composer config minimum-stability dev alongside composer config prefer-stable true. This ensures we pull in beta Symfony packages while still preferring stable dependencies where possible.

🧰 Tools
🪛 YAMLlint (1.37.1)

[warning] 39-39: too many spaces after hyphen

(hyphens)


[error] 43-43: trailing spaces

(trailing-spaces)


105-107: Matrix env fallback is intentional; static lint false positive
The expression ${{ matrix.env || 'test_cached' }} provides a default if env isn't set in the matrix. Although actionlint flags matrix.env as undefined, GitHub Actions supports this fallback syntax. You can safely ignore this warning.

🧰 Tools
🪛 actionlint (1.7.7)

105-105: property "env" is not defined in object type {mysql: number; php: number; symfony: string}

(expression)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Dependencies Pull requests that update a dependency file Maintenance CI configurations, READMEs, releases, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants