From d7b390df741bf624b233916fcec9388cf6f26ada Mon Sep 17 00:00:00 2001 From: Benson Wong Date: Wed, 14 May 2025 21:51:53 -0700 Subject: [PATCH] Add GH Action for Testing on Windows (#132) * Add windows specific test changes * Change the command line parsing library - Possible breaking changes for windows users! --- .github/workflows/go-ci-windows.yml | 26 +++++++++++++++++---- .github/workflows/go-ci.yml | 19 +++++++++++++-- Makefile | 4 ++++ go.mod | 2 +- go.sum | 2 ++ proxy/config.go | 11 +++++---- proxy/config_posix_test.go | 36 +++++++++++++++++++++++++++++ proxy/config_test.go | 28 ---------------------- proxy/config_windows_test.go | 35 ++++++++++++++++++++++++++++ proxy/helpers_test.go | 7 +++++- proxy/process_test.go | 8 ++++++- 11 files changed, 136 insertions(+), 42 deletions(-) create mode 100644 proxy/config_posix_test.go create mode 100644 proxy/config_windows_test.go diff --git a/.github/workflows/go-ci-windows.yml b/.github/workflows/go-ci-windows.yml index f178934..6cd2747 100644 --- a/.github/workflows/go-ci-windows.yml +++ b/.github/workflows/go-ci-windows.yml @@ -1,13 +1,11 @@ -# This workflow will build a golang project - name: Windows CI on: push: - branches: [ "windows-tests" ] + branches: [ "main" ] pull_request: - branches: [ "windows-tests" ] + branches: [ "main" ] # Allows manual triggering of the workflow workflow_dispatch: @@ -24,10 +22,28 @@ jobs: with: go-version: '1.23' + # cache simple-responder to save the build time + - name: Restore Simple Responder + id: restore-simple-responder + uses: actions/cache/restore@v4 + with: + path: ./build + key: ${{ runner.os }}-simple-responder-${{ hashFiles('misc/simple-responder/simple-responder.go') }} + # necessary for testing proxy/Process swapping - name: Create simple-responder + if: steps.restore-simple-responder.outputs.cache-hit != 'true' shell: bash - run: make simple-responder + run: make simple-responder-windows + + - name: Save Simple Responder + # nothing new to save ... skip this step + if: steps.restore-simple-responder.outputs.cache-hit != 'true' + id: save-simple-responder + uses: actions/cache/save@v4 + with: + path: ./build + key: ${{ runner.os }}-simple-responder-${{ hashFiles('misc/simple-responder/simple-responder.go') }} - name: Test all shell: bash diff --git a/.github/workflows/go-ci.yml b/.github/workflows/go-ci.yml index 36e30cd..c4ae20e 100644 --- a/.github/workflows/go-ci.yml +++ b/.github/workflows/go-ci.yml @@ -1,5 +1,3 @@ -# This workflow will build a golang project - name: Linux CI on: @@ -24,9 +22,26 @@ jobs: with: go-version: '1.23' + # cache simple-responder to save the build time + - name: Restore Simple Responder + id: restore-simple-responder + uses: actions/cache/restore@v4 + with: + path: ./build + key: ${{ runner.os }}-simple-responder-${{ hashFiles('misc/simple-responder/simple-responder.go') }} + # necessary for testing proxy/Process swapping - name: Create simple-responder run: make simple-responder + - name: Save Simple Responder + # nothing new to save ... skip this step + if: steps.restore-simple-responder.outputs.cache-hit != 'true' + id: save-simple-responder + uses: actions/cache/save@v4 + with: + path: ./build + key: ${{ runner.os }}-simple-responder-${{ hashFiles('misc/simple-responder/simple-responder.go') }} + - name: Test all run: make test-all \ No newline at end of file diff --git a/Makefile b/Makefile index b3f6f7e..4b1876c 100644 --- a/Makefile +++ b/Makefile @@ -46,6 +46,10 @@ simple-responder: GOOS=darwin GOARCH=arm64 go build -o $(BUILD_DIR)/simple-responder_darwin_arm64 misc/simple-responder/simple-responder.go GOOS=linux GOARCH=amd64 go build -o $(BUILD_DIR)/simple-responder_linux_amd64 misc/simple-responder/simple-responder.go +simple-responder-windows: + @echo "Building simple responder for windows" + GOOS=windows GOARCH=amd64 go build -o $(BUILD_DIR)/simple-responder.exe misc/simple-responder/simple-responder.go + # Ensure build directory exists $(BUILD_DIR): mkdir -p $(BUILD_DIR) diff --git a/go.mod b/go.mod index 407979a..d7b3ac1 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,6 @@ go 1.23.0 require ( github.com/fsnotify/fsnotify v1.9.0 github.com/gin-gonic/gin v1.10.0 - github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/stretchr/testify v1.9.0 github.com/tidwall/gjson v1.18.0 github.com/tidwall/sjson v1.2.5 @@ -13,6 +12,7 @@ require ( ) require ( + github.com/billziss-gh/golib v0.2.0 // indirect github.com/bytedance/sonic v1.11.6 // indirect github.com/bytedance/sonic/loader v0.1.1 // indirect github.com/cloudwego/base64x v0.1.4 // indirect diff --git a/go.sum b/go.sum index 6ea5c34..6a867c0 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,5 @@ +github.com/billziss-gh/golib v0.2.0 h1:NyvcAQdfvM8xokKkKotiligKjKXzuQD4PPykg1nKc/8= +github.com/billziss-gh/golib v0.2.0/go.mod h1:mZpUYANXZkDKSnyYbX9gfnyxwe0ddRhUtfXcsD5r8dw= github.com/bytedance/sonic v1.11.6 h1:oUp34TzMlL+OY1OUWxHqsdkgC/Zfc85zGqw9siXjrc0= github.com/bytedance/sonic v1.11.6/go.mod h1:LysEHSvpvDySVdC2f87zGWf6CIKJcAvqab1ZaiQtds4= github.com/bytedance/sonic/loader v0.1.1 h1:c+e5Pt1k/cy5wMveRDyk2X4B9hF4g7an8N3zCYjJFNM= diff --git a/proxy/config.go b/proxy/config.go index cb5284b..7e05c4e 100644 --- a/proxy/config.go +++ b/proxy/config.go @@ -4,11 +4,12 @@ import ( "fmt" "io" "os" + "runtime" "sort" "strconv" "strings" - "github.com/google/shlex" + "github.com/billziss-gh/golib/shlex" "gopkg.in/yaml.v3" ) @@ -233,9 +234,11 @@ func SanitizeCommand(cmdStr string) ([]string, error) { cmdStr = strings.ReplaceAll(cmdStr, "\\\n", " ") // Split the command into arguments - args, err := shlex.Split(cmdStr) - if err != nil { - return nil, err + var args []string + if runtime.GOOS == "windows" { + args = shlex.Windows.Split(cmdStr) + } else { + args = shlex.Posix.Split(cmdStr) } // Ensure the command is not empty diff --git a/proxy/config_posix_test.go b/proxy/config_posix_test.go new file mode 100644 index 0000000..7f0317d --- /dev/null +++ b/proxy/config_posix_test.go @@ -0,0 +1,36 @@ +//go:build !windows + +package proxy + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestConfig_SanitizeCommand(t *testing.T) { + // Test a command with spaces and newlines + args, err := SanitizeCommand(`python model1.py \ + -a "double quotes" \ + --arg2 'single quotes' + -s + --arg3 123 \ + --arg4 '"string in string"' + -c "'single quoted'" + `) + assert.NoError(t, err) + assert.Equal(t, []string{ + "python", "model1.py", + "-a", "double quotes", + "--arg2", "single quotes", + "-s", + "--arg3", "123", + "--arg4", `"string in string"`, + "-c", `'single quoted'`, + }, args) + + // Test an empty command + args, err = SanitizeCommand("") + assert.Error(t, err) + assert.Nil(t, args) +} diff --git a/proxy/config_test.go b/proxy/config_test.go index 6cff42d..6017f37 100644 --- a/proxy/config_test.go +++ b/proxy/config_test.go @@ -258,34 +258,6 @@ func TestConfig_FindConfig(t *testing.T) { assert.Equal(t, ModelConfig{}, modelConfig) } -func TestConfig_SanitizeCommand(t *testing.T) { - - // Test a command with spaces and newlines - args, err := SanitizeCommand(`python model1.py \ - -a "double quotes" \ - --arg2 'single quotes' - -s - --arg3 123 \ - --arg4 '"string in string"' - -c "'single quoted'" - `) - assert.NoError(t, err) - assert.Equal(t, []string{ - "python", "model1.py", - "-a", "double quotes", - "--arg2", "single quotes", - "-s", - "--arg3", "123", - "--arg4", `"string in string"`, - "-c", `'single quoted'`, - }, args) - - // Test an empty command - args, err = SanitizeCommand("") - assert.Error(t, err) - assert.Nil(t, args) -} - func TestConfig_AutomaticPortAssignments(t *testing.T) { t.Run("Default Port Ranges", func(t *testing.T) { diff --git a/proxy/config_windows_test.go b/proxy/config_windows_test.go new file mode 100644 index 0000000..72ad932 --- /dev/null +++ b/proxy/config_windows_test.go @@ -0,0 +1,35 @@ +//go:build windows + +package proxy + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestConfig_SanitizeCommand(t *testing.T) { + // does not support single quoted strings like in config_posix_test.go + args, err := SanitizeCommand(`python model1.py \ + -a "double quotes" \ + -s + --arg3 123 \ + --arg4 '"string in string"' + -c "'single quoted'" + `) + assert.NoError(t, err) + assert.Equal(t, []string{ + "python", "model1.py", + "-a", "double quotes", + "-s", + "--arg3", "123", + "--arg4", "'string in string'", // this is a little weird but the lexer says so...? + "-c", `'single quoted'`, + }, args) + + // Test an empty command + args, err = SanitizeCommand("") + assert.Error(t, err) + assert.Nil(t, args) + +} diff --git a/proxy/helpers_test.go b/proxy/helpers_test.go index 6fc9e07..0cf7c03 100644 --- a/proxy/helpers_test.go +++ b/proxy/helpers_test.go @@ -45,7 +45,12 @@ func TestMain(m *testing.M) { func getSimpleResponderPath() string { goos := runtime.GOOS goarch := runtime.GOARCH - return filepath.Join("..", "build", fmt.Sprintf("simple-responder_%s_%s", goos, goarch)) + + if goos == "windows" { + return filepath.Join("..", "build", "simple-responder.exe") + } else { + return filepath.Join("..", "build", fmt.Sprintf("simple-responder_%s_%s", goos, goarch)) + } } func getTestPort() int { diff --git a/proxy/process_test.go b/proxy/process_test.go index c73089d..885cbfd 100644 --- a/proxy/process_test.go +++ b/proxy/process_test.go @@ -5,6 +5,7 @@ import ( "net/http" "net/http/httptest" "os" + "runtime" "sync" "testing" "time" @@ -432,7 +433,12 @@ func TestProcess_ForceStopWithKill(t *testing.T) { // unexpected EOF because the kill happened, the "1" is sent before the kill // then the unexpected EOF is sent after the kill - assert.Equal(t, "1unexpected EOF\n", w.Body.String()) + if runtime.GOOS == "windows" { + assert.Contains(t, w.Body.String(), "wsarecv: An existing connection was forcibly closed by the remote host") + } else { + assert.Contains(t, w.Body.String(), "unexpected EOF") + } + close(waitChan) }()