From 57a4fb454b210dfdd1f8a59e20a5e0b5c9b3358f Mon Sep 17 00:00:00 2001 From: "e. alvarez" <55966724+ealvar3z@users.noreply.github.com> Date: Tue, 12 Sep 2023 19:07:12 -0400 Subject: [PATCH 1/6] Resolve #487: Enhance tests and fix linting issues - Add randomized fields to Engine structs in tests - Implement cryptographically secure random utility functions - Address golangci-lint warnings --- engines_test.go | 20 ++++++++++++++++++-- internal/test/random.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) create mode 100644 internal/test/random.go diff --git a/engines_test.go b/engines_test.go index 31e7ec8be..17ef565c3 100644 --- a/engines_test.go +++ b/engines_test.go @@ -8,15 +8,27 @@ import ( "testing" . "github.com/sashabaranov/go-openai" + "github.com/sashabaranov/go-openai/internal/test" "github.com/sashabaranov/go-openai/internal/test/checks" ) +// Helper function to test the ListEngines endpoint. +func RandomEngine() Engine { + return Engine{ + ID: test.RandomString(), + Object: test.RandomString(), + Owner: test.RandomString(), + Ready: test.RandomBool(), + } +} + // TestGetEngine Tests the retrieve engine endpoint of the API using the mocked server. func TestGetEngine(t *testing.T) { client, server, teardown := setupOpenAITestServer() defer teardown() server.RegisterHandler("/v1/engines/text-davinci-003", func(w http.ResponseWriter, r *http.Request) { - resBytes, _ := json.Marshal(Engine{}) + engine := RandomEngine() + resBytes, _ := json.Marshal(engine) fmt.Fprintln(w, string(resBytes)) }) _, err := client.GetEngine(context.Background(), "text-davinci-003") @@ -28,7 +40,11 @@ func TestListEngines(t *testing.T) { client, server, teardown := setupOpenAITestServer() defer teardown() server.RegisterHandler("/v1/engines", func(w http.ResponseWriter, r *http.Request) { - resBytes, _ := json.Marshal(EnginesList{}) + engines := make([]Engine, 5) + for i := range engines { + engines[i] = RandomEngine() + } + resBytes, _ := json.Marshal(EnginesList{Engines: engines}) fmt.Fprintln(w, string(resBytes)) }) _, err := client.ListEngines(context.Background()) diff --git a/internal/test/random.go b/internal/test/random.go new file mode 100644 index 000000000..d7fd9d2b6 --- /dev/null +++ b/internal/test/random.go @@ -0,0 +1,35 @@ +package test + +import ( + "crypto/rand" + "math/big" +) + +var ( + strLen = 10 + //nolint:gomnd // this avoids the golangci-lint "magic number" warning + n = big.NewInt(2) +) + +// RandomString generates a cryptographically secure random string of a fixed +// length. The string is composed of alphanumeric characters and is generated +// using the crypto/rand library. The length of the string is determined by the +// constant strLen. +func RandomString() string { + var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") + s := make([]rune, strLen) + max := big.NewInt(int64(len(letters))) + for i := range s { + randomInt, _ := rand.Int(rand.Reader, max) + s[i] = letters[randomInt.Int64()] + } + return string(s) +} + +// RandomBool generates a cryptographically secure random boolean value. It +// uses the crypto/rand library to generate a random integer (either 0 or 1), +// and returns true if the integer is 1, and false otherwise. +func RandomBool() bool { + randomInt, _ := rand.Int(rand.Reader, n) + return randomInt.Int64() == 1 +} From bd1bb2065aedd6295445d8e67310d09425e09a7c Mon Sep 17 00:00:00 2001 From: "e. alvarez" <55966724+ealvar3z@users.noreply.github.com> Date: Thu, 14 Sep 2023 17:34:28 -0400 Subject: [PATCH 2/6] Rewrite Cryptographically Secure RandomString Function - Avoid big package - Use strings.Builder for efficient string concatenation - Set default string length to 10 - Handle errors from rand.Read() Resolves: #487 --- internal/test/random.go | 52 ++++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/internal/test/random.go b/internal/test/random.go index d7fd9d2b6..eb37ba5a1 100644 --- a/internal/test/random.go +++ b/internal/test/random.go @@ -2,34 +2,42 @@ package test import ( "crypto/rand" - "math/big" + "strings" ) -var ( - strLen = 10 - //nolint:gomnd // this avoids the golangci-lint "magic number" warning - n = big.NewInt(2) -) +const letters = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" +const strLen = 10 -// RandomString generates a cryptographically secure random string of a fixed -// length. The string is composed of alphanumeric characters and is generated -// using the crypto/rand library. The length of the string is determined by the -// constant strLen. +// See StackOverflow answer: +// https://stackoverflow.com/questions/22892120/how-to-generate-a-random-string-of-a-fixed-length-in-go +// RandomString generates a cryptographically secure random string of length +// strLen. func RandomString() string { - var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") - s := make([]rune, strLen) - max := big.NewInt(int64(len(letters))) - for i := range s { - randomInt, _ := rand.Int(rand.Reader, max) - s[i] = letters[randomInt.Int64()] + sb := strings.Builder{} + sb.Grow(strLen) + + for i := 0; i < strLen; i++ { + randomByte := make([]byte, 1) + _, err := rand.Read(randomByte) + if err != nil { + return "" + } + randomIndex := randomByte[0] % byte(len(letters)) + sb.WriteByte(letters[randomIndex]) } - return string(s) + + return sb.String() } -// RandomBool generates a cryptographically secure random boolean value. It -// uses the crypto/rand library to generate a random integer (either 0 or 1), -// and returns true if the integer is 1, and false otherwise. +// RandomBool generates a cryptographically secure random boolean value. +// It reads a single byte from the crypto/rand library and uses its least +// significant bit to determine the boolean value. The function returns +// true if the least significant bit is 1, and false otherwise. func RandomBool() bool { - randomInt, _ := rand.Int(rand.Reader, n) - return randomInt.Int64() == 1 + var b [1]byte + _, err := rand.Read(b[:]) + if err != nil { + return false + } + return b[0]&1 == 1 } From d8c78aa279bbb72b2fb9bed1a5f21dabe287cb1a Mon Sep 17 00:00:00 2001 From: "e. alvarez" <55966724+ealvar3z@users.noreply.github.com> Date: Thu, 14 Sep 2023 18:01:45 -0400 Subject: [PATCH 3/6] Create RandomInt Function - Memory efficient solution for RandomInt func - Improve error handling by logging them vice returning the type - Use RandomInt() instead Resolves: #487 --- engines_test.go | 2 +- internal/test/random.go | 21 +++++++++++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/engines_test.go b/engines_test.go index 17ef565c3..69c30377f 100644 --- a/engines_test.go +++ b/engines_test.go @@ -40,7 +40,7 @@ func TestListEngines(t *testing.T) { client, server, teardown := setupOpenAITestServer() defer teardown() server.RegisterHandler("/v1/engines", func(w http.ResponseWriter, r *http.Request) { - engines := make([]Engine, 5) + engines := make([]Engine, test.RandomInt(5)) for i := range engines { engines[i] = RandomEngine() } diff --git a/internal/test/random.go b/internal/test/random.go index eb37ba5a1..babbc5ad4 100644 --- a/internal/test/random.go +++ b/internal/test/random.go @@ -2,11 +2,13 @@ package test import ( "crypto/rand" + "log" "strings" ) const letters = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" const strLen = 10 +const bitLen = 0xFF // See StackOverflow answer: // https://stackoverflow.com/questions/22892120/how-to-generate-a-random-string-of-a-fixed-length-in-go @@ -20,7 +22,7 @@ func RandomString() string { randomByte := make([]byte, 1) _, err := rand.Read(randomByte) if err != nil { - return "" + log.Fatalf("Error generating random string: %v", err) } randomIndex := randomByte[0] % byte(len(letters)) sb.WriteByte(letters[randomIndex]) @@ -29,6 +31,21 @@ func RandomString() string { return sb.String() } +// RandomInt generates a random integer between 0 (inclusive) and 'max' +// (exclusive). We uses the crypto/rand library for generating random +// bytes. It then performs a bitwise AND operation with 0xFF to keep only the +// least significant 8 bits, effectively converting the byte to an integer. The +// resulting integer is then modulo'd with 'max'. +func RandomInt(max int) int { + var b [1]byte + _, err := rand.Read(b[:]) + if err != nil { + log.Fatalf("Error generating random int: %v", err) + } + n := int(b[0]&bitLen) % max + return n +} + // RandomBool generates a cryptographically secure random boolean value. // It reads a single byte from the crypto/rand library and uses its least // significant bit to determine the boolean value. The function returns @@ -37,7 +54,7 @@ func RandomBool() bool { var b [1]byte _, err := rand.Read(b[:]) if err != nil { - return false + log.Fatalf("Error generating random bool: %v", err) } return b[0]&1 == 1 } From 6694d4b1e15d9810a8a5f7ec1cbdef0e76708f8a Mon Sep 17 00:00:00 2001 From: ealvar3z <55966724+ealvar3z@users.noreply.github.com> Date: Mon, 9 Oct 2023 17:28:40 -0700 Subject: [PATCH 4/6] Implement MaybeSeedRNG for optional deterministic tests --- engines_test.go | 1 + internal/test/random.go | 26 +++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/engines_test.go b/engines_test.go index 69c30377f..98a75368e 100644 --- a/engines_test.go +++ b/engines_test.go @@ -37,6 +37,7 @@ func TestGetEngine(t *testing.T) { // TestListEngines Tests the list engines endpoint of the API using the mocked server. func TestListEngines(t *testing.T) { + test.MaybeSeedRNG() // see docstring at internal/test/random.go client, server, teardown := setupOpenAITestServer() defer teardown() server.RegisterHandler("/v1/engines", func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/test/random.go b/internal/test/random.go index babbc5ad4..90f773c73 100644 --- a/internal/test/random.go +++ b/internal/test/random.go @@ -1,9 +1,12 @@ package test import ( - "crypto/rand" "log" + "math/rand" + "os" + "strconv" "strings" + "time" ) const letters = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" @@ -20,6 +23,7 @@ func RandomString() string { for i := 0; i < strLen; i++ { randomByte := make([]byte, 1) + // #nosec G404 _, err := rand.Read(randomByte) if err != nil { log.Fatalf("Error generating random string: %v", err) @@ -38,6 +42,7 @@ func RandomString() string { // resulting integer is then modulo'd with 'max'. func RandomInt(max int) int { var b [1]byte + // #nosec G404 _, err := rand.Read(b[:]) if err != nil { log.Fatalf("Error generating random int: %v", err) @@ -52,9 +57,28 @@ func RandomInt(max int) int { // true if the least significant bit is 1, and false otherwise. func RandomBool() bool { var b [1]byte + // #nosec G404 _, err := rand.Read(b[:]) if err != nil { log.Fatalf("Error generating random bool: %v", err) } return b[0]&1 == 1 } + +// MaybeSeedRNG optionally seeds the random number generator based on the +// TEST_RNG_SEED environment variable. If TEST_RNG_SEED is set to an integer +// value, the RNG is seeded with that value. If it's set to "random", the RNG +// is seeded using the current time. If the variable is not set or contains an +// invalid value, the RNG remains unseeded and retains its default behavior. +func MaybeSeedRNG() { + seedEnv := os.Getenv("TEST_RNG_SEED") + + if seedValue, err := strconv.ParseInt(seedEnv, 10, 64); err == nil { + rand.Seed(seedValue) + return + } + + if seedEnv == "random" { + rand.Seed(time.Now().UnixNano()) + } +} From 6b9cef4154278920a58ea831fa2d8445375f87e5 Mon Sep 17 00:00:00 2001 From: ealvar3z <55966724+ealvar3z@users.noreply.github.com> Date: Mon, 9 Oct 2023 17:38:05 -0700 Subject: [PATCH 5/6] Resolve merged conflicts from stash --- engines_test.go | 13 ++++++++--- internal/test/random.go | 50 ++++++++++++++--------------------------- 2 files changed, 27 insertions(+), 36 deletions(-) diff --git a/engines_test.go b/engines_test.go index 98a75368e..dd8b9f0ae 100644 --- a/engines_test.go +++ b/engines_test.go @@ -24,15 +24,22 @@ func RandomEngine() Engine { // TestGetEngine Tests the retrieve engine endpoint of the API using the mocked server. func TestGetEngine(t *testing.T) { + test.Seed(42) // Seed the RNG client, server, teardown := setupOpenAITestServer() defer teardown() + + expectedEngine := RandomEngine() // move outside of handler per code review comment server.RegisterHandler("/v1/engines/text-davinci-003", func(w http.ResponseWriter, r *http.Request) { - engine := RandomEngine() - resBytes, _ := json.Marshal(engine) + resBytes, _ := json.Marshal(expectedEngine) fmt.Fprintln(w, string(resBytes)) }) - _, err := client.GetEngine(context.Background(), "text-davinci-003") + actualEngine, err := client.GetEngine(context.Background(), "text-davinci-003") checks.NoError(t, err, "GetEngine error") + + // Compare the two using only one field per code review comment + if actualEngine.ID != expectedEngine.ID { + t.Errorf("Engine ID mismatch: got %s, expected %s", actualEngine.ID, expectedEngine.ID) + } } // TestListEngines Tests the list engines endpoint of the API using the mocked server. diff --git a/internal/test/random.go b/internal/test/random.go index 90f773c73..fcdbc01a5 100644 --- a/internal/test/random.go +++ b/internal/test/random.go @@ -1,7 +1,6 @@ package test import ( - "log" "math/rand" "os" "strconv" @@ -11,24 +10,26 @@ import ( const letters = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" const strLen = 10 -const bitLen = 0xFF + +// #nosec G404 +var r = rand.New(rand.NewSource(time.Now().UnixNano())) + +// Seeding func. +// #nosec G404 +func Seed(s int64) { + r = rand.New(rand.NewSource(s)) +} // See StackOverflow answer: // https://stackoverflow.com/questions/22892120/how-to-generate-a-random-string-of-a-fixed-length-in-go -// RandomString generates a cryptographically secure random string of length +// RandomString generates a random string of length // strLen. func RandomString() string { sb := strings.Builder{} sb.Grow(strLen) for i := 0; i < strLen; i++ { - randomByte := make([]byte, 1) - // #nosec G404 - _, err := rand.Read(randomByte) - if err != nil { - log.Fatalf("Error generating random string: %v", err) - } - randomIndex := randomByte[0] % byte(len(letters)) + randomIndex := r.Intn(len(letters)) sb.WriteByte(letters[randomIndex]) } @@ -36,33 +37,16 @@ func RandomString() string { } // RandomInt generates a random integer between 0 (inclusive) and 'max' -// (exclusive). We uses the crypto/rand library for generating random -// bytes. It then performs a bitwise AND operation with 0xFF to keep only the -// least significant 8 bits, effectively converting the byte to an integer. The -// resulting integer is then modulo'd with 'max'. +// (exclusive). func RandomInt(max int) int { - var b [1]byte - // #nosec G404 - _, err := rand.Read(b[:]) - if err != nil { - log.Fatalf("Error generating random int: %v", err) - } - n := int(b[0]&bitLen) % max - return n + return r.Intn(max) } -// RandomBool generates a cryptographically secure random boolean value. -// It reads a single byte from the crypto/rand library and uses its least -// significant bit to determine the boolean value. The function returns -// true if the least significant bit is 1, and false otherwise. +// RandomBool generates a random boolean value. +// #nosec G404 func RandomBool() bool { - var b [1]byte - // #nosec G404 - _, err := rand.Read(b[:]) - if err != nil { - log.Fatalf("Error generating random bool: %v", err) - } - return b[0]&1 == 1 + n := 2 // #gomnd (golangci-lint magic number suppression) + return r.Intn(n) == 1 } // MaybeSeedRNG optionally seeds the random number generator based on the From a85dd4520ea28e0fde70fee8df3a21562d9fc9ee Mon Sep 17 00:00:00 2001 From: ealvar3z <55966724+ealvar3z@users.noreply.github.com> Date: Tue, 10 Oct 2023 22:09:54 -0700 Subject: [PATCH 6/6] Remove test.Seed --- engines_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/engines_test.go b/engines_test.go index dd8b9f0ae..44f9f5d89 100644 --- a/engines_test.go +++ b/engines_test.go @@ -24,7 +24,6 @@ func RandomEngine() Engine { // TestGetEngine Tests the retrieve engine endpoint of the API using the mocked server. func TestGetEngine(t *testing.T) { - test.Seed(42) // Seed the RNG client, server, teardown := setupOpenAITestServer() defer teardown()