test: add coverage and review tooling#1
Conversation
|
Warning Ignoring CodeRabbit configuration file changes. For security, only the configuration from the base branch is applied for open source repositories. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds CI (GitHub Actions) for server and app tests, Codecov reporting, CodeRabbit auto-review configuration, expands .gitignore, and introduces multiple new unit tests in server (Go) and app (Dart) including model serialization, websocket client lifecycle, and MAC/address utilities. ChangesTesting and Coverage Infrastructure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/ci.yml (1)
33-35: ⚡ Quick winPin staticcheck to a fixed version instead of
@latest.Using
@latestmakes CI non-reproducible and can fail unexpectedly when upstream releases change behavior. Specify an explicit version (e.g.,@v1.26.0) to ensure consistent and reliable builds across environments.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 33 - 35, Replace the dynamic `@latest` pin for staticcheck in the CI workflow with a fixed semantic version to ensure reproducible CI runs; update the `go install honnef.co/go/tools/cmd/staticcheck@latest` invocation to use a specific tag (for example `@v1.26.0`) so the subsequent `"$(go env GOPATH)/bin/staticcheck"` call runs a known, locked version.server/internal/model/web_socket_model_test.go (1)
61-77: ⚡ Quick winPrefer blocking select to eliminate busy-wait polling.
The helper uses a
defaultcase with sleep, causing it to poll every 10ms. Removing thedefaultcase would let theselectblock until the channel is ready or the timeout occurs, eliminating unnecessary CPU usage.⚡ Proposed refactor to use blocking select
func waitForClosedChannel(t *testing.T, ch chan *WsSendMsg) { t.Helper() deadline := time.After(2 * time.Second) for { select { case _, ok := <-ch: if !ok { return } + // Channel had data but wasn't closed; continue waiting case <-deadline: t.Fatal("timed out waiting for writer coroutine to close channel") - default: - time.Sleep(10 * time.Millisecond) } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/internal/model/web_socket_model_test.go` around lines 61 - 77, The waitForClosedChannel helper is busy-waiting due to the default case and time.Sleep; change the select in waitForClosedChannel to a blocking select (remove the default branch) so it blocks on either receiving from ch (and returning when the channel is closed) or the deadline case which calls t.Fatal on timeout; keep the existing deadline := time.After(2 * time.Second) and use the same receive check (case _, ok := <-ch: if !ok { return }) and timeout handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.coderabbit.yaml:
- Around line 19-28: The path_instructions block is currently a top-level key
and must be nested under the reviews section; update .coderabbit.yaml so the
path_instructions mapping (the block containing firmware/**, server/**, app/**
rules) is indented under the existing reviews: key (move/indent the
path_instructions block so it becomes a child of reviews) so CodeRabbit will
apply those rules.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 33-35: Replace the dynamic `@latest` pin for staticcheck in the CI
workflow with a fixed semantic version to ensure reproducible CI runs; update
the `go install honnef.co/go/tools/cmd/staticcheck@latest` invocation to use a
specific tag (for example `@v1.26.0`) so the subsequent `"$(go env
GOPATH)/bin/staticcheck"` call runs a known, locked version.
In `@server/internal/model/web_socket_model_test.go`:
- Around line 61-77: The waitForClosedChannel helper is busy-waiting due to the
default case and time.Sleep; change the select in waitForClosedChannel to a
blocking select (remove the default branch) so it blocks on either receiving
from ch (and returning when the channel is closed) or the deadline case which
calls t.Fatal on timeout; keep the existing deadline := time.After(2 *
time.Second) and use the same receive check (case _, ok := <-ch: if !ok { return
}) and timeout handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 93d89606-4182-4ffc-a16f-b9ffbe8f7d56
⛔ Files ignored due to path filters (1)
server/go.sumis excluded by!**/*.sum
📒 Files selected for processing (12)
.coderabbit.yaml.github/workflows/ci.yml.gitignoreapp/test/model/expression_data_test.dartapp/test/util/mac_address_validator_test.dartapp/test/widget_test.dartcodecov.ymlserver/internal/model/expression_data_test.goserver/internal/model/user_test.goserver/internal/model/value_constant_test.goserver/internal/model/web_socket_model_test.goserver/internal/service/device_test.go
💤 Files with no reviewable changes (2)
- app/test/widget_test.dart
- server/internal/service/device_test.go
|
@coderabbitai Please review. |
|
✅ Actions performedReview triggered.
|
|
Closing for now. The test additions may still be useful later, but this PR also carries Codecov/CodeRabbit tooling that no longer matches the current fork direction. We can reintroduce the useful testing pieces later in a narrower form. |
Summary
Verification
Summary by CodeRabbit
Tests
Chores