Skip to content

Code coverage support#49

Open
froozeify wants to merge 9 commits into
glpi-project:v1from
froozeify:code-coverage
Open

Code coverage support#49
froozeify wants to merge 9 commits into
glpi-project:v1from
froozeify:code-coverage

Conversation

@froozeify
Copy link
Copy Markdown
Member

@froozeify froozeify commented Dec 31, 2025

This PR add code coverage support for plugin.

By default the coverage is not enabled.

For a better output, plugin can extract the report to automatically create a comment on the PR with the report in md format.

Interconnected PR :

Whatsapp plugin got a dedicated PR to test the modification (see in the feed under for the link)

Screenshot

image

Once the project is merged on main you got comparison otherwise (whatsapp branch) :

image

Test PR with simulated code into main

https://github.com/glpi-network/whatsapp-wcoverage/pull/2

Test PR targeting not the default branch (coverage should not be run)

https://github.com/glpi-network/whatsapp-wcoverage/pull/3

@froozeify froozeify marked this pull request as ready for review December 31, 2025 11:10
@froozeify froozeify changed the title Code coverage Code coverage support Dec 31, 2025
@froozeify froozeify requested review from Rom1-B and stonebuzz and removed request for AdrienClairembault and trasher December 31, 2025 14:42
Comment thread .github/workflows/continuous-integration.yml Outdated
@froozeify
Copy link
Copy Markdown
Member Author

froozeify commented Feb 9, 2026

We should create a dedicated config file. For example .glpi-coverage.json

If the file is not present or if his field enabled is false we don't run code coverage.

Enabled field is optional, value is true by default. For glpi-empty, we would be able like that to set it has example with enabled: false

This file would should contain something like

{
  "enabled": true,
  "lower_threshold": 50,
  "upper_threshold": 75,
  "fail_below_min": false
}

We should probably also create a generic action for the code coverage report config in that case. (if feasible)
This will avoid to have to put that config on every plugin : https://github.com/glpi-network/whatsapp/pull/32/changes#diff-c471496bcb3ed47397b73d2e3e3aa41a8a679c86accaed9e05676fd9d5987434R41-R70

@froozeify froozeify force-pushed the code-coverage branch 8 times, most recently from 435f715 to 91aa982 Compare February 19, 2026 15:45
@froozeify
Copy link
Copy Markdown
Member Author

@stonebuzz @Rom1-B

Small issue reported to me by @cedric-anne is that currently the new coverage comparison is done only based on the plugin default main branch artifact.

Some plugin could have 2 "main" branch one for 10.x and one for 11.x so the coverage difference could be biased.

We should see if it will have a big impact, supporting both based on PR would probably mean an even bigger ci code for check/updating those artifacts. (Probably not possible to trigger artifact refresh on target branch before running the ci and coverage on current pr, not sure github support synchronous run like that)

@Rom1-B
Copy link
Copy Markdown
Contributor

Rom1-B commented Feb 23, 2026

In that case, perhaps the simplest solution would be to run it only with the main branch? So, don't display anything with 10.0./bugfixes.

@froozeify
Copy link
Copy Markdown
Member Author

I've updated to run coverage only when the default branch is targeted.

@Rom1-B Rom1-B requested a review from cedric-anne February 23, 2026 14:51
Comment thread .github/workflows/continuous-integration.yml Outdated
Comment thread .github/workflows/continuous-integration.yml Outdated
Comment thread .github/workflows/coverage-report.yml Outdated
Comment thread .github/workflows/coverage-report.yml Outdated
Comment thread README.md Outdated
Comment thread README.md
Comment thread README.md
@Rom1-B
Copy link
Copy Markdown
Contributor

Rom1-B commented May 21, 2026

Please rebase

@froozeify
Copy link
Copy Markdown
Member Author

On it, pushed accidentally before rebase ;)

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