fix: make tests platform-agnostic for Windows support#42
Conversation
- Add CreateMockCommand helper that creates .bat files on Windows, shell scripts on Unix - Add MockPATH helper using os.PathListSeparator - Fix cmd_test.go to use platform-aware mocks instead of shell scripts - Fix config_test.go to use os.TempDir() instead of hardcoded Unix paths - Remove os.Symlink usage that requires admin on Windows - Add Windows to CI test matrix Closes #38 Signed-off-by: Mohit Nagaraj <mohitnagaraj20@gmail.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the test suite's compatibility by making it platform-agnostic, with a primary focus on enabling robust testing on Windows. It introduces new helper functions for creating mock commands and managing environment paths that adapt to the operating system, and refactors existing tests to utilize these new utilities. The overall impact is a more reliable and portable test environment, crucial for cross-platform development and ensuring consistent behavior across different operating systems. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces platform-agnostic test helper functions for creating mock executables and managing the PATH environment variable. It then refactors existing CLI command unit tests to utilize these new helpers, enhancing their robustness and cross-platform compatibility, particularly for Windows environments. The review comments suggest further refactoring in cmd_test.go to fully adopt the CreateMockCommand helper for Unix-specific mock creation, aiming for greater consistency in test logic across different operating systems.
| mockScript := `#!/bin/sh | ||
| echo "Docker Compose version v2.20.0" | ||
| ` | ||
| err := os.WriteFile(filepath.Join(suite.tempDir, "docker-compose"), []byte(mockScript), 0755) | ||
| assert.NoError(suite.T(), err) | ||
| os.WriteFile( | ||
| filepath.Join(s.tempDir, "docker-compose"), | ||
| []byte(mockScript), 0755, | ||
| ) | ||
| // Also mock docker itself | ||
| dockerMock := `#!/bin/sh | ||
| echo "Docker version 24.0.0" | ||
| ` | ||
| os.WriteFile( | ||
| filepath.Join(s.tempDir, "docker"), | ||
| []byte(dockerMock), 0755, | ||
| ) |
There was a problem hiding this comment.
To improve consistency with the Windows test logic and leverage the new helper function, you can refactor this part to use helpers.CreateMockCommand. This will make the test cleaner and more maintainable.
| mockScript := `#!/bin/sh | |
| echo "Docker Compose version v2.20.0" | |
| ` | |
| err := os.WriteFile(filepath.Join(suite.tempDir, "docker-compose"), []byte(mockScript), 0755) | |
| assert.NoError(suite.T(), err) | |
| os.WriteFile( | |
| filepath.Join(s.tempDir, "docker-compose"), | |
| []byte(mockScript), 0755, | |
| ) | |
| // Also mock docker itself | |
| dockerMock := `#!/bin/sh | |
| echo "Docker version 24.0.0" | |
| ` | |
| os.WriteFile( | |
| filepath.Join(s.tempDir, "docker"), | |
| []byte(dockerMock), 0755, | |
| ) | |
| helpers.CreateMockCommand(s.T(), s.tempDir, "docker-compose", "echo Docker Compose version v2.20.0") | |
| // Also mock docker itself | |
| helpers.CreateMockCommand(s.T(), s.tempDir, "docker", "echo Docker version 24.0.0") |
| mockScript := `#!/bin/sh | ||
| if [ "$1" = "compose" ]; then | ||
| echo "Docker Compose version v2.20.0" | ||
| elif [ "$2" = "-f" ]; then | ||
| echo "NAME STATUS PORTS" | ||
| echo "kubeorchestra-postgres running 5432/tcp" | ||
| fi | ||
| ` | ||
| err = os.WriteFile(filepath.Join(suite.tempDir, "docker-mock"), []byte(mockScript), 0755) | ||
| assert.NoError(suite.T(), err) | ||
|
|
||
| // Create symlink or copy to docker | ||
| err = os.Symlink(filepath.Join(suite.tempDir, "docker-mock"), filepath.Join(suite.tempDir, "docker")) | ||
| if err != nil { | ||
| // If symlink fails, just skip this test | ||
| suite.T().Skip("Cannot create docker mock") | ||
| os.WriteFile( | ||
| filepath.Join(s.tempDir, "docker"), | ||
| []byte(mockScript), 0755, | ||
| ) |
There was a problem hiding this comment.
To keep the test logic consistent across platforms, you can use the helpers.CreateMockCommand helper here, similar to the Windows implementation.
scriptBody := "if [ \"$1\" = \"compose\" ]; then\n" +
"\techo \"Docker Compose version v2.20.0\"\n" +
"elif [ \"$2\" = \"-f\" ]; then\n" +
"\techo \"NAME STATUS PORTS\"\n" +
"\techo \"kubeorchestra-postgres running 5432/tcp\"\n" +
"fi"
helpers.CreateMockCommand(s.T(), s.tempDir, "docker", scriptBody)1 participant
Summary
CreateMockCommandhelper that generates.batfiles on Windows, shell scripts on UnixMockPATHhelper usingos.PathListSeparatorinstead of hardcoded:cmd_test.go: uses platform-aware mocks, removesos.Symlinkdependencyconfig_test.go: usesos.TempDir()instead of hardcoded/home/user/...pathswindows-latest) to CI test matrix alongside UbuntuRelated Issue
Closes #38
Test plan