Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes integration tests to demonstrate that VM startup and initialization are working correctly. The changes add a new test execution script and modify the test setup to properly handle PATH configuration and temporary directory resolution on different platforms (particularly macOS).
Changes:
- Added new shell script
integration/test.shto build and run integration tests individually - Updated
integration/main_test.goto improve PATH setup and handle symlink resolution for temp directories - Added Makefile target
test-integrationto build test binary and execute tests using gotestsum - Updated libkrun version from v1.15.1 to v1.17.4 and added required libcap-ng-dev dependency
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| integration/test.sh | New shell script to build test binary (with macOS code signing) and execute integration tests individually |
| integration/main_test.go | Enhanced PATH setup to include executable directory and improved temp directory handling with symlink resolution |
| Makefile | Added integration test build target and test-integration target using gotestsum |
| Dockerfile | Updated libkrun to v1.17.4 and added libcap-ng-dev dependency for the build |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
d619f0b to
84a91ba
Compare
84a91ba to
53be338
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
integration/main_test.go:81
- The error messages here use capitalized "Failed to..." which is inconsistent with the existing error messages in integration/vm_test.go that use lowercase "failed to...". For consistency within the integration package, consider using lowercase error messages.
t.Fatal("Failed to get current working directory:", err)
}
vm, err := tc.vmm.NewInstance(t.Context(), resolvedTd)
if err != nil {
t.Fatal("Failed to create VM instance:", err)
}
if err := vm.Start(t.Context()); err != nil {
t.Fatal("Failed to start VM instance:", err)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add make target to run integration tests Signed-off-by: Derek McGowan <derek@mcg.dev>
Signed-off-by: Derek McGowan <derek@mcg.dev>
Signed-off-by: Derek McGowan <derek@mcg.dev>
53be338 to
cc10f23
Compare
Currently only a few tests are there, but running the integration tests is useful to show vm startup and init are working