Skip to content

avoid coverage reporting when only executing test subset #12924

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

Merged
merged 1 commit into from
Jul 29, 2025

Conversation

alexrashed
Copy link
Member

@alexrashed alexrashed commented Jul 29, 2025

Motivation

With #12923 we fixed the upload of our coverage data, which didn't work since we moved from CircleCI to GitHub actions (implemented with #12708).

Unfortunately, we now publish the coverage data also for runs where we only execute a small subset of our tests (i.e. only the acceptance tests).
This PR aims at fixing this issue.

Changes

  • Only execute the reporting step in the main pipeline if onlyAcceptanceTests and enableTestSelection are false, i.e. we also run our integration tests and do not select the tests to execute (but execute all of them).

Testing

@alexrashed alexrashed added this to the Playground milestone Jul 29, 2025
@alexrashed alexrashed added the area: ci Running LocalStack in CI environments label Jul 29, 2025
@alexrashed alexrashed added the semver: patch Non-breaking changes which can be included in patch releases label Jul 29, 2025
@alexrashed alexrashed added the review: merge when ready Signals to the reviewer that a PR can be merged if accepted label Jul 29, 2025
Copy link

github-actions bot commented Jul 29, 2025

Test Results - Preflight, Unit

21 991 tests  ±0   20 257 ✅ ±0   6m 34s ⏱️ +5s
     1 suites ±0    1 734 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 813b8fe. ± Comparison against base commit 7ebcdac.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jul 29, 2025

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 8s ⏱️ -12s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 813b8fe. ± Comparison against base commit 7ebcdac.

♻️ This comment has been updated with latest results.

Copy link
Member

@silv-io silv-io left a comment

Choose a reason for hiding this comment

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

LGTM if the action confirms it! Thank you for the fix :)

Copy link

github-actions bot commented Jul 29, 2025

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   2h 21m 31s ⏱️
4 841 tests 4 361 ✅ 480 💤 0 ❌
4 847 runs  4 361 ✅ 486 💤 0 ❌

Results for commit 813b8fe.

♻️ This comment has been updated with latest results.

@coveralls
Copy link

coveralls commented Jul 29, 2025

Coverage Status

coverage: 83.295% (+31.9%) from 51.438%
when pulling 813b8fe on fix-coveralls-report-on-partial
into 7ebcdac on main.

@alexrashed alexrashed merged commit 52c267f into main Jul 29, 2025
99 of 100 checks passed
@alexrashed alexrashed deleted the fix-coveralls-report-on-partial branch July 29, 2025 15:33
@alexrashed alexrashed mentioned this pull request Jul 31, 2025
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ci Running LocalStack in CI environments review: merge when ready Signals to the reviewer that a PR can be merged if accepted semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants