add test cases for darwin libkrun #288
Conversation
WalkthroughAdds platform-based Ginkgo label filtering to the e2e Makefile target and introduces/updates end-to-end tests: a new macOS libkrun provider test suite and additional platform labels on existing e2e specs. Changes
Sequence Diagram(s)sequenceDiagram
participant Make as Makefile
participant G as Ginkgo
participant T as TestSuite
participant P as macProvider tests
Note over Make,G: e2e target runs with platform label filter
Make->>G: ginkgo --tags e2e --v --ginkgo.label-filter=$(DEFAULT_GOOS)
G->>T: Select tests matching label (linux/darwin/windows)
alt macOS selected
T->>P: Execute `macProvider_test.go` suite
P->>P: lifecycle: init -> list -> start -> ssh -> verify -> cleanup
else other platforms
T->>T: Execute platform-appropriate suites
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
test/e2e/macProvider_test.go (1)
15-201: Consider refactoring to reduce code duplication.The test logic is nearly identical to
setup_test.go, with the main difference being the addition of--provider libkrunflags. Consider introducing helper functions or table-driven tests to reduce duplication and improve maintainability.Example approach:
- Extract common test logic into shared helper functions
- Pass provider name as a parameter
- Reuse the same test scenarios for both default and libkrun providers
This can be addressed in a future PR if preferred.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Makefile(1 hunks)test/e2e/macProvider_test.go(1 hunks)test/e2e/setup_test.go(1 hunks)test/e2e/start_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
test/e2e/setup_test.go (1)
vendor/github.com/onsi/ginkgo/v2/core_dsl.go (1)
Describe(514-516)
test/e2e/macProvider_test.go (3)
vendor/github.com/onsi/ginkgo/v2/core_dsl.go (4)
Describe(514-516)BeforeEach(726-728)AfterEach(754-756)It(573-575)vendor/github.com/onsi/gomega/gomega_dsl.go (2)
Expect(220-223)Eventually(393-396)vendor/github.com/onsi/ginkgo/v2/reporting_dsl.go (1)
CurrentSpecReport(33-35)
test/e2e/start_test.go (1)
vendor/github.com/onsi/ginkgo/v2/core_dsl.go (1)
Describe(514-516)
⏰ 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). (5)
- GitHub Check: test (windows-latest)
- GitHub Check: lint
- GitHub Check: test (macos-13)
- GitHub Check: e2e (ubuntu-24.04)
- GitHub Check: test (ubuntu-22.04)
🔇 Additional comments (5)
Makefile (1)
34-34: LGTM! Platform-specific test filtering implemented correctly.The addition of
--ginkgo.label-filter=$(DEFAULT_GOOS)enables platform-specific test execution, which aligns with the new label annotations being added across the test suite.test/e2e/setup_test.go (1)
59-59: LGTM! Platform labels added appropriately.The addition of "linux" and "darwin" labels enables these init tests to be selected by the platform-specific label filter introduced in the Makefile.
test/e2e/start_test.go (1)
9-9: LGTM! Cross-platform labels added correctly.The test validates error handling for a non-existing VM, which is platform-independent. Including all three platform labels ("windows", "linux", "darwin") ensures this test runs on all supported platforms.
test/e2e/macProvider_test.go (2)
25-55: LGTM! Cleanup logic is thorough.The AfterEach properly handles both default and named VM cleanup scenarios with the correct provider flags.
149-158: Verify the extended timeout for libkrun.The Eventually timeout is set to 20 minutes, while the equivalent test in
setup_test.go(line 202) uses 10 minutes. If libkrun has longer cloud-init completion times on macOS, this is appropriate. Otherwise, consider aligning with the existing timeout.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/e2e/vm_test.go (1)
42-42: LGTM! Platform labels correctly added.The addition of platform labels
Label("linux","darwin")correctly enables this test to run on both Linux and macOS, aligning with the PR's objective to add darwin libkrun test support. The syntax is correct for Ginkgo v2, and the test logic is platform-agnostic.Optional: Consider adding spaces after commas for consistency:
Label("linux", "darwin").
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Makefile(1 hunks)test/e2e/macProvider_test.go(1 hunks)test/e2e/setup_test.go(1 hunks)test/e2e/start_test.go(1 hunks)test/e2e/vm_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- Makefile
- test/e2e/macProvider_test.go
- test/e2e/start_test.go
- test/e2e/setup_test.go
🧰 Additional context used
🧬 Code graph analysis (1)
test/e2e/vm_test.go (1)
vendor/github.com/onsi/ginkgo/v2/core_dsl.go (1)
It(573-575)
⏰ 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). (3)
- GitHub Check: test (windows-latest)
- GitHub Check: lint
- GitHub Check: e2e (ubuntu-24.04)
1 participant
Summary by CodeRabbit
Tests
Chores