feat(test): divide unit and integration test; save coverage artiface to codecov #144
Conversation
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Justfile
participant GoTest
participant Ginkgo
participant CoverageDir
Developer->>Justfile: Run `just tests`
Justfile->>GoTest: Run unit tests with coverage
GoTest->>CoverageDir: Output coverage/unit.txt
Justfile->>Ginkgo: Run integration tests with coverage
Ginkgo->>CoverageDir: Output coverage/integration.txt
Justfile->>CoverageDir: Filter and save coverage files
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Bug: GoReleaser No Longer Checks Repository Cleanliness
The GoReleaser job no longer depends on the Dirty job, which previously ensured the repository had no uncommitted changes. This allows releases to be built from an unclean or inconsistent repository state. While the Deploy job still retains this dependency, a critical safety check for the release creation process has been removed.
.github/workflows/main.yml#L110-L112
agent/.github/workflows/main.yml
Lines 110 to 112 in 2d01dc8
Was this report helpful? Give feedback by reacting with 👍 or 👎
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Justfile (1)
27-27: Address the race flag TODO comments.The TODO comments indicate that running tests with the
-raceflag needs to be fixed. This is important for detecting race conditions in concurrent code.Would you like me to help investigate why the race flag is problematic and suggest potential solutions?
Also applies to: 36-36
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
.github/workflows/main.ymlis excluded by!**/*.ymlTaskfile.yamlis excluded by!**/*.yaml
📒 Files selected for processing (5)
.gitignore(1 hunks)Justfile(2 hunks)internal/membership_listener_test.go(1 hunks)main.go(1 hunks)tests/main_test.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Dirty
🔇 Additional comments (7)
.gitignore (1)
12-12: LGTM: Coverage directory properly ignored.Adding the
coveragedirectory to.gitignoreis appropriate since theJustfilechanges generate coverage artifacts in this directory. This prevents coverage reports from being committed to version control.main.go (1)
8-9: Centralized go:generate directives for operator validatedI’ve verified that:
- The only references to
//go:generate rm -rf ./dist/operatorand the subsequent clone are inmain.go.- No other parts of the codebase reference
dist/operator.- Existing
go:generatedirectives elsewhere are for mocks and won’t be affected.This setup won’t run during normal builds—only when
go generateis explicitly invoked—so it won’t interfere with application builds or tests.(Optional) You may want to document this behavior in your README or Makefile so that other contributors are aware that running
go generatein the root will re-clone the operator.tests/main_test.go (1)
41-42: Path update is consistent with the reorganization.The CRD directory path update from
dist/operatorto../dist/operatoris correct since the operator repository is now cloned at the root level (viamain.go). This ensures the test environment can properly locate the CRD files.internal/membership_listener_test.go (1)
44-45: Path update maintains consistency across test files.The CRD directory path update from
dist/operatorto../dist/operatoris consistent with the changes intests/main_test.goand aligns with the operator repository now being cloned at the root level.Justfile (3)
10-10: Target rename aligns with test reorganization.Renaming
tests-alltotestssimplifies the interface and aligns with the new structure where unit and integration tests are run separately but combined under a single target.
31-34: Unit test coverage configuration looks solid.The coverage setup for unit tests is well-configured:
- Creates coverage directory appropriately
- Uses atomic coverage mode for concurrent testing
- Filters out appropriate paths from coverage reports
- Saves both raw and filtered coverage files
40-43: Please verify Ginkgo flags and coverage filters in JustfileEnsure that the
ginkgo -r -p --output-interceptor-mode=none --output-dir=coverage --covermode atomic --cover --coverprofile=integration.txt --timeout "10m" --coverpkg=./internal/... ./testsinvocation uses only supported flags and that each flag is necessary for your integration test runs. Also confirm your coverage-filtering regex only excludes the intended files.• Double-check support for:
--output-interceptor-mode=none--output-dir--covermode--coverprofile--coverpkg• Validate the grep exclusion pattern:
generated|pkg|web|tests/integration|with_trace|noop
No description provided.