feat: Major Rewrite #6
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Owner
larkinwc
commented
Jun 17, 2025
larkinwc
commented
- Adds an extensive testing framework
- Fixes some execution issues that existed in the main logic
There was a problem hiding this comment.
Pull Request Overview
This PR implements a major rewrite of the integration testing framework and addresses execution issues in the core commands.
- Introduces a suite of Docker-based LXC integration test scripts (basic, mock, hybrid, advanced, etc.)
- Updates CI workflow to build and run integration tests in multiple environments
- Refactors
up/downcommands to use a default config file when none is specified
Reviewed Changes
Copilot reviewed 54 out of 54 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| integration-test/docker-lxc/run-integration-tests.sh | Adds a full end-to-end LXC integration test runner script |
| cmd/lxc-compose/up.go | Refactors up command to load default config and remove old compose conversion |
| cmd/lxc-compose/down.go | Refactors down command to accept --file flag and default config |
| .github/workflows/integration-test.yml | New CI job matrix to build, test, and publish binaries and Docker images |
Comments suppressed due to low confidence (5)
cmd/lxc-compose/up.go:33
- Remove the commented-out block that converts to
common.ComposeConfig, since it’s dead code andcfgis used directly now.
// Convert to compose config type
cmd/lxc-compose/down.go:15
- The
configFilevariable declared insideinitshadows the package-level flag. Consider renaming or hoisting it to avoid confusion.
var configFile string
integration-test/docker-lxc/run-integration-tests.sh:67
- On cleanup verification, the script only warns when containers remain but does not fail. Consider exiting with non-zero status if leftovers are detected to surface test failures.
if lxc-ls | grep -E "(web|db)"; then
integration-test/docker-lxc/mock-lxc-test.sh:41
- Piping output and using
|| truesuppresses errors. For critical workflows, capture and assert expected outputs or exit codes instead of always returning success.
LXC_MOCK_MODE=true /var/tmp/lxc-compose-mock --config lxc-compose.yml up web 2>&1 | head -10 || true
.github/workflows/integration-test.yml:5
- Pull requests against feature branches (e.g.
feat/*) won’t trigger CI becausepull_requestis only set formainanddevelop. Consider addingfix/*topull_request.branchesfor consistent testing.
branches: [ main, develop, fix/* ]
…n loading - Updated `down.go` and `up.go` to load configuration from the new `config` package instead of the deprecated `common` package. - Removed `types.go` and `device.go` from the `common` package, consolidating validation logic into the new `validation` package. - Adjusted related tests and container management code to reflect the new structure and ensure compatibility with the updated configuration handling.
- Added error logging for missing specified config files in `initConfig`. - Updated `ps` command to load and validate configuration from a specified file, defaulting to `lxc-compose.yml`. - Included configuration validation in the `up` command to ensure proper setup before container management. - Enhanced integration tests to check for invalid configuration handling. - Updated example `lxc-compose.yml` to include versioning for better compatibility.
- Introduced `test-init-fix.sh` for testing init system detection and container setup. - Added `test-simple.yml` for a simple container configuration. - Refactored container manager creation to support backend selection in `down.go`, `logs.go`, `pause.go`, `ps.go`, `unpause.go`, and `up.go`. - Implemented `manager_factory.go` to handle backend-specific container manager instantiation. - Added `pct_manager.go` as a placeholder for Proxmox `pct` commands, with basic container management functions. - Enhanced `LXCManager` with improved logging and configuration generation for better container initialization.
- Introduced `IMPLEMENTATION_STATUS.md` to document completed tasks and future priorities for Proxmox PCT integration. - Implemented `PCTManager` with full support for container management operations: Create, Remove, List, Start, Stop, etc. - Enhanced CLI with `--backend` flag for backend selection, maintaining backward compatibility with LXC. - Updated type system with new fields and extended `Manager` interface. - Established testing infrastructure for PCT manager, including basic tests for functionality and integration with CLI commands. - Refactored existing tests to accommodate changes in configuration handling and validation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.