Skip to content

Contributors cannot verify changes to the framework bridges #169

Open

Description

@derpixler

As a contributor to the PHP API client, I want changes to the Laravel and Symfony bridges to be verifiable before they are merged, so that I can tell whether my change works instead of finding out from a user's broken application.

Use case scenarios

A green build that proves nothing

Mira sends her first pull request to the client. It touches src/Bridge/SymfonyBundle.php, changing how the client is registered with the container. She runs make test locally and pushes; CI reports success. Nothing in that result relates to her change: the bundle has no tests, and phpstan.neon.dist excludes src/Bridge/ from analysis, so neither the test run nor the static analysis ever looked at the file she edited. She has no way to find out whether the DI wiring she wrote is correct, short of building a Symfony application by hand and trying it. She expected the project's own checks to tell her.

A review with nothing to lean on

Jonas reviews that pull request. For any other file in src/ he can read the accompanying test, check that PHPStan runs at level max over it, and see the diff in coverage. For the bridge he has none of that — he is left reading the DI calls and deciding from memory whether setDefinition() with an inline Definition argument behaves the way the author assumes. He approves or blocks on judgement alone, on the two files in the project where he has the least support.

No answer to "which version does this support?"

Jonas wants to add a test for the Symfony bundle and needs to know what to test against. The suggest section of composer.json names symfony/dependency-injection and symfony/http-kernel but gives no version constraint for either, and nothing else in the repository states a supported range. The bridges are documented as a supported feature in the README and in examples/cookbook/09-symfony.php, so users reasonably assume some range is supported — but there is nothing to write a test matrix against.


Agreed Solution & Architecture Decision

To keep maintenance simple while keeping downstream release archives lightweight, we will keep the framework bridges inside this repository rather than splitting them into separate repos.

  1. Distribution Footprint (.gitattributes): Add export-ignore rules for tests/, .github/, docs/, and tooling configs to keep composer require zammad/zammad-api-client-php minimal.
  2. In-Repo Integration Testing: Require framework components in require-dev and build dedicated integration tests under tests/Bridge/Symfony/ (using a minimal TestKernel).
  3. Static Analysis & CI: Remove static analysis exclusions (phpstan.neon.dist) for src/Bridge/ and test against supported Symfony versions in CI.

Acceptance criteria

  • A change that breaks the Symfony bridge's container wiring causes CI to fail.
    • Verifying the bridge does not require a running Zammad instance — it exercises DI wiring, not HTTP.
  • Static analysis covers the Symfony bridge at the same level as the rest of src/.
    • The excludePaths entry for it in phpstan.neon.dist is removed.
  • The Symfony bundle's compile-time failure on a missing token can be fixed with confidence that the replacement wiring actually works.
  • The coverage report no longer counts the bridges as a gap that nobody is able to close.
  • The Symfony versions the bridge supports are stated in composer.json (suggest / require-dev) and tested in CI.
  • A follow-up issue exists for the Laravel bridge, covering the same outcomes.

Implementation Tasks

  • Package Footprint: Add .gitattributes to exclude tests/, .github/, docs, and dev configs from Composer archives.
  • Dev Dependencies: Add symfony/dependency-injection and symfony/http-kernel (supported versions) to require-dev in composer.json.
  • Static Analysis: Remove src/Bridge/ exclusion from phpstan.neon.dist and ensure PHPStan runs cleanly at Level Max.
  • Testing: Implement tests/Bridge/Symfony/TestKernel.php and integration tests asserting observable container behavior (service registration, lazy definitions, alias resolution).
  • Fix Regression: Resolve the missing-token compile-time failure using lazy definitions.
  • CI Matrix: Update GitHub Actions matrix to run bridge tests against supported Symfony versions.
  • Laravel Follow-up: Create a tracking issue for the Laravel bridge.

Contributions are very welcome! If you would like to help resolve this issue, please leave a comment below before opening a PR.

Activity

  1. added theissue type on Oct 12, 2026
  2. chii0815 commented on Oct 30, 2026

    @chii0815
    Contributor

    Just move the bridges to another repo and add PHPUnit for these repos and create some Github Actions.
    It would also help with the install process as the dev could do composer req zammad/zammad-api-bundle the bundle would then depent on zammad/zammad-api-client and symfony flex would configure the symfony instance correctly. this could also be done in github actions for this separate repo so it could be tested like any other symfony bundle. As for how to manage to inform the dev what your min symfony version is: This is handled by composer and symfony flex while installing the bundle. Just configure composer to
    "require": { "php": ">=8.2", "symfony/framework-bundle": "^7.4 || ^8.0" },
    that would result in an info for the dev that his version is not supported if this version is older than this.
    As 7.4.22 is the current LTSR and 8.1.10 is the current stable this would work on any newer symfony instances,
    Symfony Best Practices for Reusable Bundes

    As my current employer uses an old symfony app on version 4.4.x and the old zammad-api-client with an wrapper and we want to redo this app, I am interested in the symfony bundle and will try to create one if I got the time for it.

    For testing the zammad_client inside the bundle or /and the lib you could provide an mock-server. Twitch did the same for their api. If this is done correctly it should be possible to use this in Github Actions.

  3. derpixler commented on Nov 1, 2026

    @derpixler
    ContributorAuthor

    Thanks for raising this issue!

    We’ve updated the issue description above to reflect our decision on this topic. In short, we plan to keep the Symfony bundle within this repository while avoiding extra overhead and keeping release sizes minimal:

    1. Distribution footprint (.gitattributes): Add export-ignore rules for tests/, .github/, docs, and tooling configs so Composer releases remain strictly limited to production code.
    2. In-repo Bridge Testing: Keep Symfony bundle tests directly within this repository under tests/Bridge/Symfony/ to ensure full test coverage and static analysis (PHPStan).

    Contributions are very welcome! If anyone would like to take this on and submit a PR with the .gitattributes configuration and the TestKernel setup, feel free to drop a comment here and open a PR!


    Issue changes

    @@ -17,2 +17,11 @@ Jonas wants to add a test for the Symfony bundle and needs to know what to test
     
    +---
    +
    +## Agreed Solution & Architecture Decision
    +
    +To keep maintenance simple while keeping downstream release archives lightweight, **we will keep the framework bridges inside this repository** rather than splitting them into separate repos.
    +
    +1. **Distribution Footprint (`.gitattributes`):** Add `export-ignore` rules for `tests/`, `.github/`, `docs/`, and tooling configs to keep `composer require zammad/zammad-api-client-php` minimal.
    +2. **In-Repo Integration Testing:** Require framework components in `require-dev` and build dedicated integration tests under `tests/Bridge/Symfony/` (using a minimal `TestKernel`).
    +3. **Static Analysis & CI:** Remove static analysis exclusions (`phpstan.neon.dist`) for `src/Bridge/` and test against supported Symfony versions in CI.
    +
    +---
    +
     ## Acceptance criteria
    @@ -23,3 +32,3 @@ Acceptance criteria
     - The coverage report no longer counts the bridges as a gap that nobody is able to close
    -- The Symfony versions the bridge supports are stated somewhere a contributor will find them
    +- The Symfony versions the bridge supports are stated in `composer.json` (`suggest` / `require-dev`) and tested in CI
     - A follow-up issue exists for the Laravel bridge, covering the same outcomes
    @@ -37,13 +46,12 @@ The same gap reaches beyond `src/Bridge/`. `symfony/http-client`, `slim/slim` a
     
    -## Possible solutions
    -
    -These are ideas, not decisions — the team owns the solution.
    -
    -- Add `symfony/dependency-injection` and `symfony/http-kernel` to `require-dev` and drop the matching `excludePaths` entry. Dev dependencies do not reach users, so this costs installation time in CI and nothing else.
    -- Alternatively keep them out of `require-dev` and add a separate CI job that installs them on demand, if keeping the default dev install small is worth the extra workflow.
    -- Tests could assert the observable container behaviour: that the extension registers `ZammadClient` and the `zammad_client` alias, that resolving the service returns a working client, and that the configuration precedence (explicit config, then `$_ENV`, then the built-in default) holds.
    -- If the bridges stay outside the test run for now, excluding them from the coverage configuration would at least stop the metric reporting a gap that cannot be acted on.
    -- Version constraints could be added to the `suggest` entries, so the supported range is stated in the same place the packages are named.
    -
    -## Clarification needed
    +## Implementation Tasks
    
    -1. Which Symfony major versions should the bridge support and be tested against? For the pending fix described in Background a single version is enough to pin the wiring down; the matrix is a separate question about what the bridge promises to support.
    -2. Should the Laravel follow-up be created now as a linked issue, or only once the Symfony side is done?
    -3. Should the `suggest` entries for the other optional packages (`symfony/http-client`, `symfony/serializer`, `symfony/cache`) also gain version constraints, or is that a separate concern?
    +- [ ] **Package Footprint:** Add `.gitattributes` to exclude `tests/`, `.github/`, docs, and dev configs from Composer archives.
    +- [ ] **Dev Dependencies:** Add `symfony/dependency-injection` and `symfony/http-kernel` (supported versions) to `require-dev` in `composer.json`.
    +- [ ] **Static Analysis:** Remove `src/Bridge/` exclusion from `phpstan.neon.dist` and ensure PHPStan runs cleanly at Level Max.
    +- [ ] **Testing:** Implement `tests/Bridge/Symfony/TestKernel.php` and integration tests asserting observable container behavior (service registration, lazy definitions, alias resolution).
    +- [ ] **Fix Regression:** Resolve the missing-token compile-time failure using lazy definitions.
    +- [ ] **CI Matrix:** Update GitHub Actions matrix to run bridge tests against supported Symfony versions.
    +- [ ] **Laravel Follow-up:** Create a tracking issue for the Laravel bridge.
    +
    +---
    +
    +> **Contributions are very welcome!** If you would like to help resolve this issue, please leave a comment below before opening a PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions