Refactor: Improve serialfinder logic and robustness#2
Open
mrchypark wants to merge 6 commits into
Open
Conversation
This commit introduces significant refactoring to the serialfinder package, enhancing its robustness, maintainability, and consistency across macOS, Linux, and Windows.
Key changes include:
1. **macOS (`serialfinder_darwin.go`):**
* Improved `ioreg` output parsing for better reliability and clarity.
* Enhanced error handling for `ioreg` command execution.
* Refined VID/PID extraction and ensured case-insensitive comparison.
2. **Linux (`serialfinder_linux.go`):**
* Corrected VID/PID filtering logic to properly handle empty filters (match all) and ensure matches when filters are provided.
* Improved error handling: Critical errors during device property reading are now returned instead of just printed.
* Implemented case-insensitive VID/PID comparisons.
* Clarified that the `Port` field refers to the stable `/dev/serial/by-id` path.
3. **Windows (`serialfinder_windows.go`):**
* Replaced fragile string splitting with robust regular expressions for VID/PID extraction from device instance IDs in the registry.
* Overhauled VID/PID filtering to be case-insensitive and to correctly handle empty filters.
* Streamlined the `iterateSerialsWindows` helper function.
4. **General Improvements:**
* Standardized uppercase VID/PID handling (storage and comparison) across all platforms.
* Reviewed and updated comments for better clarity and accuracy.
* Ensured consistent population of the `SerialDeviceInfo` struct.
These changes address potential bugs in filtering, improve error reporting, and make the codebase more resilient to variations in system outputs, particularly on macOS and Windows.
This commit introduces unit tests for the serialfinder package across macOS, Linux, and Windows platforms. To enable effective testing, the existing platform-specific code was refactored to use interfaces for OS interactions (command execution, filesystem access, registry access), allowing these interactions to be mocked.
Key changes:
1. **Refactoring for Testability:**
* `serialfinder_darwin.go`: Introduced a `commandExecutor` interface to mock `ioreg` calls. `GetSerialDevices` now uses an internal testable version.
* `serialfinder_linux.go`: Introduced a `fileSystemReader` interface to mock filesystem operations. `GetSerialDevices` and helper functions now use internal testable versions.
* `serialfinder_windows.go`: Introduced `registryHandler`, `registryKey` interfaces, and a `portCheckerFunc` variable to mock registry access and COM port checks. `GetSerialDevices` and helpers now use internal testable versions.
* Default implementations of these interfaces use the standard OS library calls.
2. **Unit Tests for macOS (`serialfinder_darwin_test.go`):**
* Tests for helper functions (`parseHexValue`, `parseStringValue`).
* Comprehensive tests for `getSerialDevicesWithExecutor` using a mock `commandExecutor` to simulate various `ioreg` outputs, including:
* No devices, single/multiple devices.
* Filtering by VID/PID (case-insensitive).
* Error handling for `ioreg` command failures.
* Handling of incomplete or malformed device data from `ioreg`.
3. **Unit Tests for Linux (`serialfinder_linux_test.go`):**
* Tests for helper functions (`checkForVIDPIDFilesWithReader`, `findSerialDeviceInfoDirWithReader`).
* Comprehensive tests for `getSerialDevicesWithReader` using a `mockFileSystemReader` to simulate various filesystem states in `/dev/serial/by-id` and `/sys/class/tty`, including:
* No devices, single/multiple devices.
* Filtering by VID/PID (case-insensitive).
* Error handling for filesystem operation failures.
* Handling of missing or malformed device property files.
4. **Unit Tests for Windows (`serialfinder_windows_test.go`):**
* Tests for VID/PID regular expressions.
* Comprehensive tests for `getSerialDevicesWithRegistry` using `mockRegistryHandler`, `mockRegistryKey`, and a mock `portCheckerFunc` to simulate various Windows Registry states and COM port activity, including:
* No devices, single/multiple devices.
* Filtering by VID/PID (case-insensitive).
* Error handling for registry operation failures.
* Handling of inactive COM ports and missing registry values (e.g., PortName).
These unit tests significantly improve the code quality and provide greater confidence in the reliability and correctness of the serialfinder package across all supported platforms.
This commit introduces a GitHub Actions workflow to automatically build and test the Go project. The workflow (`.github/workflows/go_test.yml`) includes the following features: - Triggers on push and pull request events to the `main` or `master` branches. - Utilizes a matrix strategy to test on `ubuntu-latest`, `macos-latest`, and `windows-latest` runners. - Sets up Go environment (currently configured for Go 1.19.x). - Checks out the repository code. - Executes `go test -v ./...` to run all unit tests, respecting platform-specific build tags. This CI setup will help ensure code quality and catch regressions early by running tests automatically for every relevant change.
This commit updates the Go version used in the GitHub Actions CI workflow from `1.19.x` to `1.23.x` to align with project requirements.
Resolves build failures on the Darwin platform caused by: - Duplicate import statements for 'bytes' and 'fmt' in serialfinder_darwin.go. - Unused import statements for 'bytes' and 'fmt' in serialfinder_darwin_test.go. These issues were identified from CI build logs. Removing the redundant and unused imports allows the code to compile correctly.
The parseStringValue function in serialfinder_darwin.go would panic with a "slice bounds out of range" error if the input string, after trimming, was too short to be sliced (e.g., length 1) but still triggered the quote-stripping logic. This commit adds a length check (len(value) >= 2) to the condition for stripping surrounding quotes, ensuring that the slice operation value[1:len(value)-1] is only attempted on strings that are long enough. This resolves the panic observed when testing.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This commit introduces significant refactoring to the serialfinder package, enhancing its robustness, maintainability, and consistency across macOS, Linux, and Windows.
Key changes include:
macOS (
serialfinder_darwin.go):ioregoutput parsing for better reliability and clarity.ioregcommand execution.Linux (
serialfinder_linux.go):Portfield refers to the stable/dev/serial/by-idpath.Windows (
serialfinder_windows.go):iterateSerialsWindowshelper function.General Improvements:
SerialDeviceInfostruct.These changes address potential bugs in filtering, improve error reporting, and make the codebase more resilient to variations in system outputs, particularly on macOS and Windows.