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

Create merged coverage to html #1318

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Create merged coverage to html #1318

wants to merge 4 commits into from

Conversation

amit-62
Copy link

@amit-62 amit-62 commented Jan 26, 2024

It generatescoverage.out as an artifact and merges it to coverage.html.
fixes #1256

Signed-off-by: Amit Kumar <kramit6662@gmail.com>
@amit-62 amit-62 requested a review from a team as a code owner January 26, 2024 10:15
Signed-off-by: Amit Kumar <kramit6662@gmail.com>
uses: actions/checkout@v3

- name: Run Tests
run: ./run-tests.sh 6.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't make much sense. vm-test has a test matrix that runs this against a wide range of kernel versions, and we need test coverage for each of them.

The point is to add coverage reporting to the existing vm-test, test-on-prev-go and test-on-arm64 jobs. Ideally, this would all be done in a single step to keep the size of the yaml a bit manageable:

      - run: go tool covdata merge -o coverage-merged.out
      - run: go tool cover -html=coverage-merged.out -o coverage.html

      - name: Upload Coverage Report
        uses: actions/upload-artifact@v3
        with:
          name: coverage-report
          path: |
            ./coverage-merged.out
            ./coverage.html

Copy link
Author

Choose a reason for hiding this comment

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

Will I have to add this to all three job or it can be done in another separate job?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we want coverage for all three jobs (we do), then yes, it needs to be in all three. 🙂

@amit-62
Copy link
Author

amit-62 commented Feb 1, 2024

Hello @ti-mo, I have got an error about input directories, from which it will merge data from multiple dirs. What should I use for input directories?

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.

CI: store merged coverage as an artifact
2 participants