Skip to content

ci: replace with check-extension - #6

Merged
damienwebdev merged 2 commits into
mage-os-lab:mainfrom
damienwebdev:main
Jun 24, 2026
Merged

ci: replace with check-extension#6
damienwebdev merged 2 commits into
mage-os-lab:mainfrom
damienwebdev:main

Conversation

@damienwebdev

@damienwebdev damienwebdev commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Replace the bespoke .github/workflows/ci.yml with the graycoreio check-extension reusable workflow (@v8.5.0), driven by a compute_matrix job that pulls the currently-supported Magento versions. This covers unit tests, integration tests, phpcs, and DI compile across the supported version matrix.
  • Keep the checks check-extension does not provide as standalone jobs: static (PHPStan) and infection (PR-only mutation testing).
  • Drop the standalone phpcs and php-cs-fixer steps — phpcs is now handled by the reusable workflow's coding-standard action

Motivation

Replace hand-maintained CI matrix/install logic with the shared, versioned graycoreio/github-actions-magento2 workflow so supported-version coverage and Magento install steps stay current automatically, while preserving project-specific static analysis and mutation testing.

How to test

  1. Open this PR and confirm the Actions run triggers compute_matrix, check-extension (matrixed across the resolved Magento versions), static, and infection.
  2. Verify check-extension runs unit + integration tests, phpcs, and DI compile and passes.
  3. Confirm static (PHPStan) and infection (mutation testing, PR-only) run and pass.

Checklist

  • Branch is based on the latest main.
  • Commits follow Conventional Commits (feat:, fix:, refactor:, test:, chore:, docs:).
  • vendor/bin/phpunit --testsuite unit passes locally.
  • vendor/bin/phpstan analyse --memory-limit=1G passes locally.
  • vendor/bin/phpcs --standard=phpcs.xml.dist passes locally.
  • New PHP files start with declare(strict_types=1);. (n/a — no PHP files changed)
  • User-facing change has a CHANGELOG.md entry under ## [Unreleased]. (n/a — CI-only change)
  • No raw integer IDs in admin UX (use pickers / linked names. See CONTRIBUTING.md). (n/a — no UX change)

Screenshots / GraphQL samples (if UI or API change)

n/a — CI configuration change only.

@damienwebdev

Copy link
Copy Markdown
Collaborator Author

@rhoerr

The reusable workflow graycoreio/github-actions-magento2/.github/workflows/check-extension.yaml@v8.5.0 is not allowed in mage-os-lab/module-blog because all reusable workflows must be from a repository owned by mage-os-lab or match one of the patterns: Ilshidur/action-discord@, devops-infra/action-pull-request@, dorny/paths-filter@, easingthemes/ssh-deploy@, google-github-actions/release-please-action@, hashicorp/setup-terraform@, mage-os/, peter-evans/create-pull-request@, php-actions/composer@, shimataro/ssh-key-action@, shivammathur/setup-php@, tj-actions/changed-files@.

@rhoerr

rhoerr commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Stand by

@rhoerr

rhoerr commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Added graycoreio/github-actions-magento2@*, as an allowed action source. May need to authorize others as well if it invokes child actions.

@damienwebdev damienwebdev reopened this Jun 2, 2026
@damienwebdev

damienwebdev commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator Author

@rhoerr

graycoreio/github-actions-magento2@* doesnt match

  • graycoreio/github-actions-magento2/supported-version@main
  • graycoreio/github-actions-magento2/.github/workflows/integration.yaml@main

We probably need:

graycoreio/github-actions-magento2/*@main

Sorry for my evil mono-repo style :)

@rhoerr

rhoerr commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Updated

@damienwebdev damienwebdev reopened this Jun 2, 2026
@damienwebdev

Copy link
Copy Markdown
Collaborator Author

@rhoerr bam. We're live.

@DavidLambauer

Copy link
Copy Markdown
Collaborator

Fixed the mutation testing failure in eb653e9PostsByAssignmentProvider uses ResourceConnection/raw SQL so all its mutants escaped, pulling MSI below 54 %. Added it to the infection excludes alongside the other DB-dependent classes. Next CI run should be green.

@DavidLambauer

Copy link
Copy Markdown
Collaborator

Two more fixes on main:\n\n- 65dc692composer.json: widened magento/framework from ^103.0.7^103.0 so 2.4.6-p15 (which ships 103.0.6-p15) resolves. That was blocking compile, unit, and integration on the whole 2.4.6 row.\n- 2096d15phpcs.xml.dist: added warning-severity=0 so phpcs exits 0 on docblock warnings (typed PHP 8.2 methods don't need them). Only actual errors fail the build now.\n\nPush a new commit or rebase to trigger a fresh run.

@damienwebdev

damienwebdev commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator Author

@DavidLambauer

Copy link
Copy Markdown
Collaborator

Two fixes just landed on main to unblock the failing checks:

742ce4etest(integration): use Bootstrap::getObjectManager() in StorefrontRoutingTest (fixes #8)
AbstractController::$this->_objectManager returns null for module interfaces on 2.4.7-p10 due to a version-specific DI initialisation difference. Switched to the Bootstrap::getObjectManager() singleton that every other integration test in the module already uses.

d07861ffix(cs): resolve all phpcs warnings blocking the coding-standard CI job (fixes #9)
The check-extension.yaml@v8.5.0 coding-standard action runs phpcs without ignore_warnings_on_exit. These severity-≥6 warnings were causing exit code 2:

  • Empty catch blocks → phpcs:ignore (all are intentional race-condition guards)
  • Lines >120 chars → wrapped in Config.php, three Controller/Adminhtml files, and AlgorithmicLoader.php
  • Useless override → phpcs:ignore on the four Ui DataProvider constructors (they set module-specific defaults)
  • Interface naming (CollisionChecker) → phpcs:ignore for now, rename tracked separately
  • Control-structure formatting in AdminAuthorization → auto-fixed by phpcbf
  • is_file() in template plugin → phpcs:ignore

phpcs now exits 0 locally and 64 unit tests still pass.

To unblock the PR: please sync/rebase this branch onto the updated main so the CI run picks up both commits. The cancelled integration_test jobs on 2.4.6-p15 and 2.4.8-p5 are blocked by the upstream graycoreio/github-actions-magento2#287 sandbox-upload bug — nothing we can do on our side until that's patched upstream.

@damienwebdev

Copy link
Copy Markdown
Collaborator Author

@DavidLambauer one more small nit:

FILE: /home/runner/work/module-blog/module-blog/Controller/Rss/Index.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
--------------------------------------------------------------------------------
 61 | WARNING | The use of function htmlspecialchars() is discouraged; use
    |         | \Magento\Framework\Escaper->escapeHtml() instead
 66 | WARNING | The use of function htmlspecialchars() is discouraged; use
    |         | \Magento\Framework\Escaper->escapeHtml() instead
 75 | WARNING | The use of function htmlspecialchars() is discouraged; use
    |         | \Magento\Framework\Escaper->escapeHtml() instead
--------------------------------------------------------------------------------

@damienwebdev

Copy link
Copy Markdown
Collaborator Author

Unfortunately @DavidLambauer

Cannot assign null to property MageOS\Blog\Test\Integration\Controller\StorefrontRoutingTest::$repository of type MageOS\Blog\Api\PostRepositoryInterface

Still happens

@rhoerr

rhoerr commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Ping @DavidLambauer

@rhoerr

rhoerr commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Unfortunately @DavidLambauer

Cannot assign null to property MageOS\Blog\Test\Integration\Controller\StorefrontRoutingTest::$repository of type MageOS\Blog\Api\PostRepositoryInterface

Still happens

Is this resolved as of #10 being merged? Are there any blockers to this PR?

@damienwebdev

Copy link
Copy Markdown
Collaborator Author

@rhoerr #10 didn't fix this:

Cannot assign null to property MageOS\Blog\Test\Integration\Controller\StorefrontRoutingTest::$repository of type MageOS\Blog\Api\PostRepositoryInterface

@Sental

Sental commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

I think this just needs the 8.6 version of the CI instead of the 8.5 version and it should pass. @damienwebdev

@rhoerr

rhoerr commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

CI still fails with the same error after updating to 8.7.

etc/di.xml does define this type:

    <preference for="MageOS\Blog\Api\PostRepositoryInterface"
                type="MageOS\Blog\Model\PostRepository"/>

which suggests to me
(a) this probably isn't the right solution (granted it didn't solve this anyway)

        $this->repository = $objectManager->get(PostRepositoryInterface::class)
            ?? $objectManager->get(PostRepository::class);

(b) something about the integration tests or maybe CI is mishandling something about class types or autoloading.

I notice this is the first Bootstrap::getObjectManager()->get call in the first Test/Integration class that's (probably) processed. I suspect the same would happen with all other instances in the integration suite. Possibly the integration tests were written for the old CI and this bootstrapping technique doesn't work for check-extension.

@damienwebdev

damienwebdev commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author
/** @var ObjectManager */
    private $objectManager;

    public function setUp(): void
    {
        $this->objectManager = ObjectManager::getInstance();
    }

This is what I do in my tests.

Is

Bootstrap::getObjectManager();

Common?

Sental added a commit that referenced this pull request Jun 24, 2026
PR: Fix Tests to allow PR #6 to pass & merge
I'm merging this directly to enable 6 to pass
@Sental
Sental marked this pull request as draft June 24, 2026 19:13
@Sental
Sental marked this pull request as ready for review June 24, 2026 19:13

@rhoerr rhoerr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎉

@damienwebdev
damienwebdev merged commit e3b198b into mage-os-lab:main Jun 24, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants