-
Notifications
You must be signed in to change notification settings - Fork 14
Conversation
WalkthroughThe project's build process and testing suite have been updated. Variable definitions in the Makefile were refined to distinguish between all packages and unit test packages. The test commands were split to allow separate execution for all tests and unit tests. The coverage tool has been adjusted accordingly. Code organization seems to have shifted, with changes to import paths reflecting a new directory structure. The GitHub Actions workflow was updated to align with these changes, now specifically running unit tests. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (5)
- .github/workflows/test.yml (1 hunks)
- Makefile (3 hunks)
- celestia/celestia_test.go (1 hunks)
- celestia_test.go (1 hunks)
- cmd/celestia-da/server.go (1 hunks)
Files skipped from review due to trivial changes (1)
- cmd/celestia-da/server.go
Additional comments: 6
.github/workflows/test.yml (1)
- 35-35: The change from
make test
tomake test-unit
in the GitHub Actions workflow is consistent with the PR's objective to run only unit tests on commits.Makefile (4)
5-8: The introduction of
all_pkgs
andunit_pkgs
variables in theMakefile
is a good practice for distinguishing between all packages and those for unit testing.34-34: The update of the
go-acc
command in thecover
target to useunit_pkgs
aligns with the new testing strategy.70-73: The
test
target has been correctly updated to run all tests.76-79: The addition of the
test-unit
target for running unit tests is consistent with the PR's objective to separate unit tests from integration tests.celestia_test.go (1)
- 21-21: The update to the import path in
celestia_test.go
is consistent with the reorganization of the package structure.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #43 +/- ##
==========================================
- Coverage 76.23% 0.00% -76.24%
==========================================
Files 1 1
Lines 101 101
==========================================
- Hits 77 0 -77
- Misses 17 101 +84
+ Partials 7 0 -7 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- .github/workflows/test.yml (2 hunks)
Additional comments: 2
.github/workflows/test.yml (2)
35-35: The change from
make test
tomake test-unit
correctly reflects the PR's objective to separate unit tests from integration tests.52-53: The conditional check to run integration tests only on
workflow_dispatch
events is a good approach to ensure that integration tests are reserved for releases, as intended by the PR.
Overview
The current CI pipeline runs integration tests on all commits, this requires coordination between the dependencies go-da and local-celestia-devnet. The fact that the integration test depends on local-celestia-devnet which itself depends on working tree celestia-da complicates this process.
This PR separates unit tests and integration tests so that the CI pipeline can only run unit tests and all tests, including integration tests can be run on releases.
Checklist
Summary by CodeRabbit
New Features
Refactor
Chores