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

Added CodeCov coverage report support #322

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

connorhu
Copy link
Contributor

@connorhu connorhu commented Feb 13, 2024

Pull Request

Related issue

Fixes #284

What does this PR do?

  • Integrates CodeCov

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

@connorhu connorhu force-pushed the feature/284-codecov-integration branch from eafa94d to e329c12 Compare February 13, 2024 16:09
Copy link
Collaborator

@norkunas norkunas left a comment

Choose a reason for hiding this comment

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

For consistency use single quotes

.github/workflows/tests.yml Outdated Show resolved Hide resolved
@connorhu connorhu force-pushed the feature/284-codecov-integration branch from 5c62003 to 89d139e Compare February 13, 2024 18:02
Copy link

codecov bot commented Feb 13, 2024

Welcome to Codecov 🎉

Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment.

Thanks for integrating Codecov - We've got you covered ☂️

@connorhu connorhu force-pushed the feature/284-codecov-integration branch from 2a4121f to e8baa67 Compare February 13, 2024 18:18
@connorhu
Copy link
Contributor Author

connorhu commented Feb 13, 2024

We need to lock phpunit version because coverage report is broken with PHP 7.4, Symfony 5.4 and phpunit 8. With this constellation, code coverage did not work. One solution is to lock the phpunit version, another is to create a .ci/github/phpunit/phpunit.xml.dist file and lock the version only in that file. Both solutions have their advantages and disadvantages.

PHP Fatal error:  Uncaught TypeError: Argument 1 passed to SebastianBergmann\CodeCoverage\CodeCoverage::__construct() must be an instance of SebastianBergmann\CodeCoverage\Driver\Driver, null given, called in /home/runner/work/meilisearch-symfony/meilisearch-symfony/vendor/bin/.phpunit/phpunit-8.5-0/src/TextUI/TestRunner.php on line 526 and defined in /home/runner/work/meilisearch-symfony/meilisearch-symfony/vendor/phpunit/php-code-coverage/src/CodeCoverage.php:122
Stack trace:
#0 /home/runner/work/meilisearch-symfony/meilisearch-symfony/vendor/bin/.phpunit/phpunit-8.5-0/src/TextUI/TestRunner.php(526): SebastianBergmann\CodeCoverage\CodeCoverage->__construct()
#1 /home/runner/work/meilisearch-symfony/meilisearch-symfony/vendor/bin/.phpunit/phpunit-8.5-0/src/TextUI/Command.php(235): PHPUnit\TextUI\TestRunner->doRun()
#2 /home/runner/work/meilisearch-symfony/meilisearch-symfony/vendor/bin/.phpunit/phpunit-8.5-0/src/TextUI/Command.php(194): PHPUnit\TextUI\Command->run()
#3 /home/runner/work/meilisearch-symfony/meilisearch-symfony in /home/runner/work/meilisearch-symfony/meilisearch-symfony/vendor/phpunit/php-code-coverage/src/CodeCoverage.php on line 122
Script SYMFONY_DEPRECATIONS_HELPER='ignoreFile=./tests/baseline-ignore' ./vendor/bin/simple-phpunit --colors=always --verbose handling the test:unit event returned with error code 255

@connorhu connorhu force-pushed the feature/284-codecov-integration branch from 40a5f27 to a760792 Compare February 13, 2024 18:38
@norkunas
Copy link
Collaborator

I don't mind locking it as long as tests runs and are green :)

@connorhu connorhu changed the title added run coverage step Added CodeCov coverage report support Feb 13, 2024
@connorhu connorhu marked this pull request as ready for review February 13, 2024 18:54
@norkunas
Copy link
Collaborator

The CI breaks now because bors expects these jobs:

status = [
    'integration-tests (PHP 7.4) (Symfony 5.4.*)',
    'integration-tests (PHP 8.0) (Symfony 6.0.*)',
    'integration-tests (PHP 8.1) (Symfony 6.1.*)',
    'integration-tests (PHP 8.2) (Symfony 6.2.*)',
    'integration-tests (PHP 8.3) (Symfony 6.3.*)',
    'integration-tests (PHP 8.3) (Symfony 6.4.*)',
    'integration-tests (PHP 8.3) (Symfony 7.0.*)',
]

So I think either restore using with .* and then just strip it before passing to codecov to avoid limitations as you said (will be less changes) or update the bors.toml

@connorhu connorhu force-pushed the feature/284-codecov-integration branch 2 times, most recently from 6f03376 to f6c39c7 Compare February 14, 2024 06:03
@connorhu
Copy link
Contributor Author

connorhu commented Feb 14, 2024

Both solutions are good. If editing bors.toml causes further problems, I'll put an wildcard in the name of the job instead.

@norkunas
Copy link
Collaborator

The less changes are better :)

@norkunas
Copy link
Collaborator

Error: No coverage reports found. Please make sure you're generating reports successfully.
Warning: Codecov: 
                      Failed to properly upload report: The process '/home/runner/work/_actions/codecov/codecov-action/v4/dist/codecov' failed with exit code 1

@connorhu
Copy link
Contributor Author

Now it looks like it will find it:

[2024-02-14T08:05:45.455Z] ['info'] => Found 5 possible coverage files:
  phpunit-7.4-5.4-coverage/coverage.xml
  phpunit-8.0-6.0-coverage/coverage.xml
  phpunit-8.1-6.1-coverage/coverage.xml
  phpunit-8.2-6.2-coverage/coverage.xml
  phpunit-8.3-7.0-coverage/coverage.xml

@norkunas
Copy link
Collaborator

Why it's not working with v4? Adding a new feature should use last versions 🤔

@connorhu connorhu force-pushed the feature/284-codecov-integration branch from 10fd1ec to 307eeaa Compare February 14, 2024 08:12
@connorhu
Copy link
Contributor Author

Why it's not working with v4? Adding a new feature should use last versions 🤔

The problem was caused by the fact that I only transferred to v4 on download, not on upload. v3 uploaded, v4 downloaded.

@connorhu
Copy link
Contributor Author

Shiny 86% reported. Nice.
report

@norkunas
Copy link
Collaborator

I still suggest to revert changes about wildcard, because in repo settings jobs are made as required with previous names, and I don't have permissions to do that

@connorhu connorhu force-pushed the feature/284-codecov-integration branch from b95327b to a49234f Compare February 14, 2024 08:24
norkunas
norkunas previously approved these changes Feb 14, 2024
Copy link
Collaborator

@norkunas norkunas left a comment

Choose a reason for hiding this comment

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

Good job 🎉

After #323 CI will be green (I can't review/merge my own PR's)
Before merging this we should probably wait for @brunoocasali approval.

@connorhu
Copy link
Contributor Author

I signed up for that PR. If it went in I'll rebase this and we'll green it.

@connorhu
Copy link
Contributor Author

About this php-cs-fixer problem: I think you should hardcode the php-cs-fixer version everywhere, and apply the fixer changes with version changes. Elsewhere I just ran into that two versions interpreting something completely differently (incorrectly) and it took me some time to find out what the source of the problem was.

@norkunas
Copy link
Collaborator

That's just because @Symfony ruleset rules has changed, it doesn't change too much, so therefore I'm not sure about locking cs fixer

@norkunas
Copy link
Collaborator

@connorhu you can rebase now :)

@connorhu connorhu force-pushed the feature/284-codecov-integration branch from d016093 to 1b787a0 Compare February 14, 2024 16:44
@norkunas
Copy link
Collaborator

bors merge

1 similar comment
@norkunas
Copy link
Collaborator

bors merge

@norkunas
Copy link
Collaborator

bors cancel

@norkunas
Copy link
Collaborator

@connorhu could you squash your commits?

Co-authored-by: Tomas Norkūnas <norkunas.tom@gmail.com>
@connorhu connorhu force-pushed the feature/284-codecov-integration branch from 1b787a0 to 38ffced Compare February 15, 2024 10:03
@connorhu
Copy link
Contributor Author

@norkunas of course. done

@norkunas norkunas merged commit 797055d into meilisearch:main Feb 15, 2024
11 checks passed
@norkunas
Copy link
Collaborator

Thank you @connorhu

@connorhu connorhu deleted the feature/284-codecov-integration branch February 15, 2024 11:04
@curquiza curquiza added the maintenance Anything related to maintenance (CI, tests, refactoring...) label Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Anything related to maintenance (CI, tests, refactoring...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add CodeCov
3 participants