Skip to content

Test: CI Validation for SilverStripe 5 Upgrade#132

Open
muskie9 wants to merge 3 commits into5.x-devfrom
test/ci-validation
Open

Test: CI Validation for SilverStripe 5 Upgrade#132
muskie9 wants to merge 3 commits into5.x-devfrom
test/ci-validation

Conversation

@muskie9
Copy link
Copy Markdown
Member

@muskie9 muskie9 commented Nov 5, 2025

Purpose

This PR is opened to validate the CI/CD pipeline for the SilverStripe 5 upgrade.

Changes in 5.x-dev

  • ✅ Updated composer.json for SS5 compatibility (PHP 8.1+)
  • ✅ Refactored FoxyHelper to use composition instead of inheritance
  • ✅ Made GridFieldGroupable optional (SS4-only dependency)
  • ✅ Updated CI workflow for PHP 8.1/8.2/8.3 and MySQL 8.0
  • ✅ Updated PHPUnit configuration to PHPUnit 9.5

CI Testing

This PR will trigger the automated test suite to verify:

  • Composer dependencies install successfully
  • PHPUnit tests pass on PHP 8.1, 8.2, 8.3
  • PHPCS code standards check
  • Code coverage generation

Expected Results

Please review the CI results when the workflow completes. Any failures should be documented and addressed before merging the 5.x-dev branch.

Links

  • Base branch: 5.x-dev
  • CI Workflow: Will appear in Actions tab
  • Module documentation: See README.md

Note: This is a test PR to validate CI configuration. Once tests pass, this can be closed and 5.x-dev can be considered ready for testing.

@muskie9 muskie9 requested a review from Copilot November 5, 2025 21:59
@muskie9
Copy link
Copy Markdown
Member Author

muskie9 commented Nov 5, 2025

@github-actions[bot] Please review this PR and provide feedback on:

  1. CI Test Failures - Analyze any failing tests and suggest fixes
  2. Composer Dependencies - Review if all SS5 dependencies are correct
  3. Code Quality - Check for deprecated API usage that needs updating
  4. Best Practices - Suggest improvements for SS5 compatibility

This is a SilverStripe 4 to 5 upgrade. Key changes:

  • PHP 8.1+ requirement
  • SilverStripe 5.x dependencies
  • Refactored FoxyHelper class
  • Updated CI workflow for modern PHP versions

Please help identify any issues preventing the tests from passing.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds documentation for SilverStripe 5 compatibility work. It introduces a new markdown file describing the SS5 upgrade initiative and CI pipeline testing.

  • Adds a new documentation file to track SilverStripe 5 upgrade progress

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@muskie9
Copy link
Copy Markdown
Member Author

muskie9 commented Nov 5, 2025

Current Status

CI Tests: ⏳ Running (PHP 8.1, 8.2, 8.3)
Copilot Review: ✅ Submitted (but needs more context on the full 5.x-dev changes)

What This PR Actually Tests

This PR merges test/ci-validation into 5.x-dev to trigger the GitHub Actions workflow. The real changes are already on the 5.x-dev branch:

Key Changes on 5.x-dev:

  1. composer.json - Updated to PHP ^8.1, SilverStripe ^5.0, fixed dependency versions
  2. FoxyHelper.php - Refactored from inheritance to composition pattern
  3. Purchasable.php - Made GridFieldGroupable optional (SS4-only dependency)
  4. .github/workflows/ci.yml - Updated for PHP 8.1/8.2/8.3 and MySQL 8.0
  5. phpunit.xml.dist - Modernized to PHPUnit 9.5 schema

Awaiting Results

Once CI completes, we'll check for:

  • ✅ Composer install success
  • ✅ PHPUnit test results across PHP versions
  • ✅ PHPCS code standards
  • ✅ Code coverage generation

Any failures will be addressed before merging or closing this PR.

@muskie9 muskie9 requested a review from Copilot November 5, 2025 22:07
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Replace custom workflow with silverstripe/gha-ci@v1
- Matches pattern used in dynamic/silverstripe-tasks
- Provides maintained, standard CI configuration
- Automatically handles PHP versions, dependencies, and tests

This simplifies maintenance and ensures CI stays up-to-date
with SilverStripe best practices.
@muskie9
Copy link
Copy Markdown
Member Author

muskie9 commented Nov 5, 2025

🔧 Updated CI Workflow

Just pushed a significant improvement to the CI configuration:

What Changed

Replaced our custom CI workflow with SilverStripe's official shared workflow (silverstripe/gha-ci@v1).

Why This is Better

Maintained by SilverStripe - Automatically stays current with best practices
Battle-tested - Used across all official SilverStripe modules
Simpler - 12 lines instead of 75+ lines of configuration
Consistent - Matches dynamic/silverstripe-tasks and other modern modules
Auto-configured - Handles PHP versions, dependencies, MySQL, PHPUnit, PHPCS automatically

What Was Removed

  • Custom PHP matrix configuration
  • Manual MySQL service setup
  • Explicit PHPUnit and PHPCS steps
  • Old .editorconfig, .gitattributes, .gitignore, .scrutinizer.yml, .travis.yml files

Testing

The new workflow should now run and provide comprehensive CI results. This will help us identify any real issues with the SS5 upgrade.


Commit: a6b5a08
Pattern source: dynamic/silverstripe-tasks/.github/workflows/ci.yml

@muskie9 muskie9 requested a review from Copilot November 5, 2025 22:19
@muskie9
Copy link
Copy Markdown
Member Author

muskie9 commented Nov 5, 2025

@copilot-pull-request-reviewer[bot]

We've now switched to using SilverStripe's official shared CI workflow (silverstripe/gha-ci@v1).

Please analyze:

  1. Is this workflow compatible with our module structure and dependencies?
  2. What issues might arise from using the shared workflow versus our previous custom one?
  3. Are there any missing configuration files (like .gitattributes or phpcs.xml.dist) that the shared workflow expects?
  4. Do we need a specific composer.json setup for the shared workflow to work correctly?

The shared workflow is very simple and delegates everything to SilverStripe's centralized CI configuration. Please review if our module meets the requirements for this approach.

Context: This module is being upgraded from SilverStripe 4 to 5, and we want to ensure our CI setup follows modern SilverStripe best practices.

Copy link
Copy Markdown

Copilot AI commented Nov 5, 2025

@muskie9 I've opened a new pull request, #133, to work on those changes. Once the pull request is ready, I'll request review from you.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

.editorconfig:1

  • The .editorconfig file is being removed entirely. This file provides consistent coding styles across different editors and IDEs for the project. Removing it may lead to inconsistent formatting, especially since the project contains multiple file types (PHP, YAML, JS, SCSS) with different indentation requirements. Consider keeping this file to maintain code style consistency across the team.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- PHPUnit 9.5 requires explicit void return type for setUp()
- Updated PurchasableTest::setUp() to match SS5/PHPUnit 9 requirements
- Call parent::setUp() first (best practice)
- Fixes: 'Declaration of setUp() must be compatible with SapphireTest::setUp(): void'

This resolves the PHP fatal error when running tests.
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.

3 participants