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

Fix error in Code Coverage workflow #2282

Merged

Conversation

chatton
Copy link
Contributor

@chatton chatton commented Sep 13, 2022

Description

When the e2e tests were introduced, the e2e tests were excluded from the unit tests in an incorrect fashion, which resulted in each task running all tests as opposed to splitting up the tests into 4 sections.

The workflow will be skipped on this PR as no go files are changed. See the this workflow which ran with the changes to see the run times.

closes: #2167


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

@chatton chatton changed the title Split build e2e into separate task Fix error in Code Coverage workflow Sep 13, 2022
@@ -103,7 +103,7 @@ jobs:
if: env.GIT_DIFF
- name: test & coverage report creation
run: |
cat pkgs.txt.part.${{ matrix.part }} | xargs go test $(go list ./... | grep -v e2e) -race -mod=readonly -timeout 30m -coverprofile=${{ matrix.part }}profile.out -covermode=atomic -tags='ledger test_ledger_mock'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was not reading the split input from the file, but was actually building a full list of all tests to run.

Copy link
Contributor

@crodriguezvega crodriguezvega left a comment

Choose a reason for hiding this comment

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

Awesome, @chatton! I guess this also fixes this issue?

@chatton
Copy link
Contributor Author

chatton commented Sep 14, 2022

@crodriguezvega yes it looks like a duplicate issue, I've closed the original and updated the closes in the description! Thanks for pointing it out.

Copy link
Contributor

@damiannolan damiannolan left a comment

Choose a reason for hiding this comment

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

Nice! 🙏

Copy link
Contributor

@colin-axner colin-axner left a comment

Choose a reason for hiding this comment

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

Excellent catch! You are the man!

@chatton chatton merged commit 57e518c into main Sep 16, 2022
@chatton chatton deleted the cian/issue#2281-investigate-long-times-of-code-coverage-task branch September 16, 2022 09:01
mergify bot pushed a commit that referenced this pull request Sep 16, 2022
mergify bot pushed a commit that referenced this pull request Sep 16, 2022
crodriguezvega pushed a commit that referenced this pull request Sep 16, 2022
(cherry picked from commit 57e518c)

Co-authored-by: Cian Hatton <[email protected]>
crodriguezvega added a commit that referenced this pull request Sep 20, 2022
(cherry picked from commit 57e518c)

Co-authored-by: Cian Hatton <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>
Co-authored-by: Damian Nolan <[email protected]>
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.

code coverage action is taking very long
4 participants