Skip to content

Conversation

@PROFeNoM
Copy link
Contributor

@PROFeNoM PROFeNoM commented Jan 12, 2024

Description

This PR brings back code coverage using codecov.

  • CODECOV_TOKEN added to CircleCI > Project Settigns > Environment Variables
  • Add the CODECOV_TOKEN project environment variable to the docker image
  • Generate a codecov PR comment
  • Create different flags for appsec & the tracer (extension)
  • Consider adding integrations (+ corresponding flag) coverage

You can see a sample report below.

One limitation of this coverage is that functions written in the extension but solely tested in PHP (I don't think of any out of my head, but hypothetically) would be considered uncovered.

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@PROFeNoM PROFeNoM changed the title [WIP] Coverage Re-enable code coverage using codecov Jan 12, 2024
@PROFeNoM PROFeNoM self-assigned this Jan 12, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jan 12, 2024

Codecov Report

Merging #2464 (9c32ab3) into master (26f13b8) will increase coverage by 0.40%.
Report is 1249 commits behind head on master.
The diff coverage is n/a.

❗ Current head 9c32ab3 differs from pull request most recent head 5726c5b. Consider uploading reports for the commit 5726c5b to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2464      +/-   ##
============================================
+ Coverage     79.32%   79.73%   +0.40%     
- Complexity        0      216     +216     
============================================
  Files           125      104      -21     
  Lines         12713    12669      -44     
============================================
+ Hits          10085    10102      +17     
+ Misses         2628     2567      -61     
Flag Coverage Δ
tracer-extension 79.73% <ø> (?)
tracer-integrations 79.86% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 180 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba0cf68...5726c5b. Read the comment docs.

@PROFeNoM PROFeNoM added the ci label Jan 12, 2024
@PROFeNoM PROFeNoM mentioned this pull request Jan 12, 2024
2 tasks
@PROFeNoM PROFeNoM marked this pull request as ready for review January 16, 2024 10:37
@PROFeNoM PROFeNoM requested a review from a team as a code owner January 16, 2024 10:37
name: Install CodeCov Uploader Dependencies
command: |
sudo apt update
sudo apt install -y gpg
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note: add gpg to base buster container.
Maybe just include it already in dockerfile.

Comment on lines +1513 to +1514
pattern: "^[^.]+$"
value: << parameters.resource_class >>
Copy link
Collaborator

Choose a reason for hiding this comment

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

We test ... that no dot is in the resource name because they're prefixed arm.... right.

Makefile Outdated
Comment on lines 951 to 955
else \
echo "Running tests without coverage"; \
echo "$(TEST_EXTRA_ENV) $(ENV_OVERRIDE) php $(TEST_EXTRA_INI) $(REQUEST_INIT_HOOK) $(PHPUNIT) $(1) --filter=$(FILTER)"; \
$(TEST_EXTRA_ENV) $(ENV_OVERRIDE) php $(TEST_EXTRA_INI) $(REQUEST_INIT_HOOK) $(PHPUNIT) $(1) --filter=$(FILTER); \
fi
Copy link
Collaborator

Choose a reason for hiding this comment

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

spaces as indents here?

Copy link
Collaborator

@bwoebi bwoebi left a comment

Choose a reason for hiding this comment

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

Looking good :-)

@pr-commenter
Copy link

pr-commenter bot commented Jan 16, 2024

Benchmarks

Benchmark execution time: 2024-01-16 16:19:21

Comparing candidate commit 5726c5b in PR branch alex/fix/coverage with baseline commit ba0cf68 in branch master.

Found 0 performance improvements and 1 performance regressions! Performance is the same for 40 metrics, 49 unstable metrics.

scenario:PHPRedisBench/benchRedisBaseline

  • 🟥 execution_time [+172.070µs; +341.458µs] or [+6.286%; +12.473%]

@PROFeNoM PROFeNoM merged commit 32251e3 into master Jan 16, 2024
@PROFeNoM PROFeNoM deleted the alex/fix/coverage branch January 16, 2024 16:36
@github-actions github-actions bot added this to the 0.97.0 milestone Jan 16, 2024
@PROFeNoM PROFeNoM mentioned this pull request Jan 17, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants