Skip to content
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

[11.x] Add Coveralls CI workflow for test coverage reporting #53648

Closed
wants to merge 5 commits into from

Conversation

joelbladt
Copy link

@joelbladt joelbladt commented Nov 24, 2024

Description

This Pull Request introduces a new GitHub Actions workflow to integrate Coveralls for automated test coverage reporting. The workflow is added under .github/workflows/coveralls.yml. It is designed to provide detailed code coverage metrics for pull requests and the main branches.

Benefit to End Users

The inclusion of Coveralls enhances the development workflow by providing:

  • Transparent Code Coverage Metrics: Developers and contributors gain insights into the impact of their changes on the overall code coverage.
  • Improved Code Quality: Encourages maintaining and improving code coverage across the framework.
  • Better Collaboration: Simplifies identifying areas of insufficient test coverage during code reviews.

These benefits ensure a more robust and reliable Laravel framework for developers and end users alike.

Implementation Details

  1. Workflow Name: coveralls
  2. Triggers: The workflow runs on:
  • Push events to the master branch and branches matching the *.x pattern.
  • All pull requests.
  1. Jobs Configuration:
  • Runs on ubuntu-24.04 with a matrix of PHP versions (8.2, 8.3, 8.4) and PHPUnit versions (10.5.12, 11.3.2).
  • Executes tests with phpunit and generates a Clover coverage report.
  • Uploads the coverage data to Coveralls using the php-coveralls library.

The workflow does not interfere with existing configurations and complements the current CI/CD processes.

Why It Doesn't Break Existing Features

  • The workflow operates independently of existing testing and deployment pipelines.
  • No changes were made to the core functionality or APIs of the Laravel framework.
  • The coveralls.yml file is isolated and only concerns reporting test coverage metrics.

How It Makes Building Web Applications Easier

By integrating Coveralls, contributors can:

  • Ensure that new code is properly tested and covered.
  • Quickly identify areas in the framework that need more rigorous testing.
  • Enhance trust and confidence in the stability of the framework with every new change.

Tests

No additional feature tests are required for this change since it pertains to CI workflow. However:

  • The workflow has been tested locally and on other repositories to ensure compatibility with the defined configurations and tools (PHP, PHPUnit, Coveralls).
  • Verified that coverage reports are generated and uploaded successfully to Coveralls.

@joelbladt joelbladt changed the base branch from master to 10.x November 24, 2024 12:43
@joelbladt joelbladt changed the base branch from 10.x to 11.x November 24, 2024 12:44
@joelbladt joelbladt changed the title Add Coveralls CI workflow for test coverage reporting [11.x] Add Coveralls CI workflow for test coverage reporting Nov 24, 2024
fail-fast: true
matrix:
php: [ 8.2, 8.3, 8.4 ]
phpunit: [ '10.5.12', '11.3.2' ]
Copy link
Member

Choose a reason for hiding this comment

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

It should just run on a single PHP version and the latest PHPUnit.

tools: composer:v2
coverage: none
coverage: xdebug
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
coverage: xdebug
coverage: none

@@ -77,9 +77,9 @@ jobs:
uses: shivammathur/setup-php@v2
with:
php-version: 8.3
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, :php-psr
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, xml, xdebug, :php-psr
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, xml, xdebug, :php-psr
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, :php-psr

tools: composer:v2
coverage: none
coverage: xdebug
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
coverage: xdebug
coverage: none

@@ -31,9 +31,9 @@ jobs:
uses: shivammathur/setup-php@v2
with:
php-version: 8.3
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, :php-psr
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, xml, xdebug, :php-psr
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, xml, xdebug, :php-psr
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, :php-psr

tools: composer:v2
coverage: none
coverage: xdebug
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
coverage: xdebug
coverage: none

@@ -128,9 +128,9 @@ jobs:
uses: shivammathur/setup-php@v2
with:
php-version: 8.3
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, :php-psr
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, xml, xdebug, :php-psr
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, xml, xdebug, :php-psr
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, :php-psr

tools: composer:v2
coverage: none
coverage: xdebug
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
coverage: xdebug
coverage: none

@@ -82,9 +82,9 @@ jobs:
uses: shivammathur/setup-php@v2
with:
php-version: 8.3
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, :php-psr
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, xml, xdebug, :php-psr
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, xml, xdebug, :php-psr
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, :php-psr

tools: composer:v2
coverage: none
coverage: xdebug
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
coverage: xdebug
coverage: none

@@ -35,9 +35,9 @@ jobs:
uses: shivammathur/setup-php@v2
with:
php-version: 8.3
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, :php-psr
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, xml, xdebug, :php-psr
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, xml, xdebug, :php-psr
extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, pdo_mysql, :php-psr

@driesvints
Copy link
Member

While I think your intentions are well, I don't think this PR should be merged. Coverage only brings so much benefit without having its downsides, most notable runtime. This workflow will become to longest running workflow of the CI suite a thus unnecessarily slowing down feedback loops on the framework repo. I do not feel this repo will benefit from code coverage that much that it warrants the shorter feedback loop.

Scherm­afbeelding 2024-11-25 om 12 25 26

@taylorotwell
Copy link
Member

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If applicable, please consider releasing your code as a package so that the community can still take advantage of your contributions!

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