test minio in CircleCI

[jbsmith7741]

PR Type

Tests, Enhancement

---

Description

• Migrate MinIO tests from public server to local Docker instance

• Remove MinIO tests from main file package, consolidate in dedicated module

• Add CircleCI MinIO service container with environment configuration

• Update test credentials to use environment variables with local defaults

---

Diagram Walkthrough

flowchart LR
  A["Public MinIO<br/>play.minio.io"] -->|Remove| B["Local Docker<br/>MinIO Service"]
  C["file_test.go<br/>MinIO tests"] -->|Move| D["minio/glob_test.go<br/>Dedicated tests"]
  E["CircleCI Config"] -->|Add| B
  F["Environment Variables"] -->|Configure| B

Loading

File Walkthrough

	Relevant files
Tests	file_test.go Remove MinIO tests from main file package                                file/file_test.go Remove all MinIO/S3 client initialization and bucket management code Remove TestGlob_Minio test function and related helper functions Simplify test setup to only handle local file operations Remove minio-go and credentials package imports +3/-98    client_test.go Configure tests for local MinIO Docker instance                    file/minio/client_test.go Replace hardcoded public MinIO credentials with environment variable lookups Update test endpoint from play.minio.io:9000 to localhost:9000 Change Secure flag from true to false for local HTTP instance Expand test file setup to include glob test files alongside read test files Improve skip messages with colored output and clearer instructions Refactor file creation loops for better maintainability +48/-25  glob_test.go Add dedicated MinIO glob test module                                          file/minio/glob_test.go Create new dedicated test file for MinIO glob functionality Implement TestGlob with environment variable configuration Define glob test cases matching previous TestGlob_Minio patterns Use local MinIO endpoint and credentials from environment variables +64/-0
Configuration changes	config.yml Add MinIO service container to CircleCI pipeline                  .circleci/config.yml Add MinIO service container to test job with server command Configure MinIO root user credentials via environment variables Set MINIO_ENDPOINT, MINIO_ACCESS_KEY, MINIO_SECRET_KEY environment variables Replace executor reference with inline Docker configuration +12/-1

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Default credentials Description: The CI job provisions a MinIO service with trivial static credentials (MINIO_ROOT_USER/MINIO_ROOT_PASSWORD and propagated MINIO_ACCESS_KEY/MINIO_SECRET_KEY all set to minioadmin), which can be abused if the service becomes reachable from other containers/jobs or if this pattern is copied to non-test environments. config.yml [11-22] Referred Code docker: - image: cimg/go:1.25 - image: minio/minio command: server /data environment: MINIO_ROOT_USER: minioadmin MINIO_ROOT_PASSWORD: minioadmin working_directory: ~/task-tools environment: MINIO_ENDPOINT: localhost:9000 MINIO_ACCESS_KEY: minioadmin MINIO_SECRET_KEY: minioadmin
	Insecure MinIO transport Description: Test code defaults to MINIO_ACCESS_KEY/MINIO_SECRET_KEY of minioadmin and forces Secure: false (HTTP), which can lead to credentials and data being sent unencrypted if the endpoint is not strictly local/isolated or if these defaults are reused outside CI. client_test.go [19-24] Referred Code var testOption = Option{ Host: getEnv("MINIO_ENDPOINT", "localhost:9000"), AccessKey: getEnv("MINIO_ACCESS_KEY", "minioadmin"), SecretKey: getEnv("MINIO_SECRET_KEY", "minioadmin"), Secure: false, // local Docker instance uses HTTP }
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Ignored errors: The test setup and file writer operations ignore potential errors (e.g., os.Getwd(), os.MkdirAll(), w.WriteLine(), and w.Close()), reducing failure visibility and making debugging harder. Referred Code wd, _ = os.Getwd() // create local test directories and files pths := []string{ "test/file-1.txt", "test/file2.txt", "test/file3.gz", "test/f1/file4.gz", "test/f3/file5.txt", "test/f5/file-6.txt", "test/other/name/z1/file1.txt", } os.MkdirAll("./test/f1", 0750) os.MkdirAll("./test/f3", 0750) os.MkdirAll("./test/f5", 0750) os.MkdirAll("./test/other/name/z1", 0750) os.MkdirAll("./test/other/name/z2", 0750) for _, pth := range pths { if err := createFile("./"+pth, nil); err != nil { log.Fatal(err) } ... (clipped 5 lines) Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Hardcoded credentials: The PR introduces default MinIO credentials (minioadmin/minioadmin) in test configuration and environment fallbacks, which may constitute committed secrets unless strictly scoped to ephemeral local/CI-only containers. Referred Code - image: minio/minio command: server /data environment: MINIO_ROOT_USER: minioadmin MINIO_ROOT_PASSWORD: minioadmin working_directory: ~/task-tools environment: MINIO_ENDPOINT: localhost:9000 MINIO_ACCESS_KEY: minioadmin MINIO_SECRET_KEY: minioadmin Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Consolidate test setup for MinIO The new file/minio/glob_test.go is in a different package (minio_test) than its setup code in file/minio/client_test.go (minio), causing tests to fail. Move them to the same package to share the TestMain setup function. Examples: file/minio/glob_test.go [1-64] package minio_test import ( "os" "testing" "github.com/hydronica/trial" "github.com/pcelvng/task-tools/file" ) ... (clipped 54 lines) file/minio/client_test.go [33-76] func TestMain(m *testing.M) { var err error log.SetFlags(log.Llongfile) // test client testClient, err = newTestClient() if err != nil { fmt.Println("\033[34mSKIP: Minio server not available - run 'docker run -p 9000:9000 minio/minio server /data' for local testing\033[0m") return } ... (clipped 34 lines) Solution Walkthrough: Before: // file/minio/client_test.go package minio func TestMain(m *testing.M) { // ... client setup ... // Creates files needed for glob tests globFiles := []string{ "mc://.../glob/file-1.txt", ... } for _, pth := range globFiles { createTestFile(pth) } m.Run() // ... cleanup ... } // file/minio/glob_test.go package minio_test // Different package func TestGlob(t *testing.T) { // This test will fail because the files it expects // are created in the 'minio' package's TestMain, // which does not run for the 'minio_test' package. ... } After: // file/minio/client_test.go package minio func TestMain(m *testing.M) { // ... client setup ... // Creates files needed for glob tests globFiles := []string{ "mc://.../glob/file-1.txt", ... } for _, pth := range globFiles { createTestFile(pth) } m.Run() // ... cleanup ... } // file/minio/glob_test.go package minio // Same package func TestGlob(t *testing.T) { // This test will now pass because by being in the 'minio' package, // it shares the TestMain from client_test.go, ensuring // the required files are created before the test runs. ... } Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical flaw where the new glob test will fail because its setup logic in client_test.go is in a different Go package and will not be executed.	High
Possible issue	Add endpoint to file.Options In file/minio/glob_test.go, update file.Options to include the Host endpoint and set Secure: false to enable connection to the local MinIO test server. file/minio/glob_test.go [25-28] opts := &file.Options{ + Host: endpoint, AccessKey: accessKey, SecretKey: secretKey, + Secure: false, } Apply / Chat Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies that the Host and Secure fields are missing from file.Options, which would cause the test to fail. Applying this change is critical for the test's correctness.	High
	Expose and wait for MinIO In the CircleCI configuration, expose the MinIO container's port for clarity and consider adding a health-check step to ensure the service is ready before tests run. .circleci/config.yml [13-17] - image: minio/minio command: server /data + ports: + - 9000:9000 environment: MINIO_ROOT_USER: minioadmin MINIO_ROOT_PASSWORD: minioadmin Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that exposing the MinIO port is good practice for clarity, although not strictly necessary for service-to-service communication within the same Docker network.	Medium
General	Properly skip tests in TestMain In TestMain, replace return with os.Exit(0) when the Minio server is unavailable to ensure the test suite exits immediately with a success status. file/minio/client_test.go [39-42] if err != nil { fmt.Println("\033[34mSKIP: Minio server not available - run 'docker run -p 9000:9000 minio/minio server /data' for local testing\033[0m") - return + os.Exit(0) } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that return from TestMain does not prevent other tests from running and proposes using os.Exit(0) for a clean skip, which is a valid improvement.	Medium
	Avoid code duplication by centralizing helper To avoid code duplication, move the getEnv function from file/minio/client_test.go and file/minio/glob_test.go into a single shared test utility file. file/minio/glob_test.go [11-16] -func getEnv(key, fallback string) string { +// This function should be removed from this file and moved to a shared location. +// For example, create a new file `file/minio/test_helper.go` with: +/* +package minio + +import "os" + +func GetEnv(key, fallback string) string { if value := os.Getenv(key); value != "" { return value } return fallback } +*/ +// Then, in this file, you would call `minio.GetEnv(...)` after importing the `minio` package. Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that the getEnv function is duplicated in file/minio/client_test.go and file/minio/glob_test.go and proposes a valid refactoring to improve code maintainability.	Low
Learned best practice	Defer cleanup and check errors Add defer cleanups immediately after successful bucket/object creation so early returns don't leak resources, and check/log errors from rmTestFile/rmBucket instead of ignoring them. file/minio/client_test.go [44-92] // make test bucket if err := createBucket(testBucket); err != nil { fmt.Printf("\033[34mSKIP: error creating bucket: %v\033[0m\n", err) return } +defer func() { + if err := rmBucket(testBucket); err != nil { + log.Printf("cleanup: failed to remove bucket %q: %v", testBucket, err) + } +}() // create test files for reading readFiles := []string{ fmt.Sprintf("mc://%v/read/test.txt", testBucket), fmt.Sprintf("mc://%v/read/test.gz", testBucket), } for _, pth := range readFiles { if err := createTestFile(pth); err != nil { fmt.Printf("\033[34mSKIP: error creating %s: %v\033[0m\n", pth, err) return } } +defer func() { + for _, pth := range readFiles { + if err := rmTestFile(pth); err != nil { + log.Printf("cleanup: failed to remove object %q: %v", pth, err) + } + } +}() // create test files for glob testing globFiles := []string{ fmt.Sprintf("mc://%v/glob/file-1.txt", testBucket), fmt.Sprintf("mc://%v/glob/file2.txt", testBucket), fmt.Sprintf("mc://%v/glob/file3.gz", testBucket), fmt.Sprintf("mc://%v/glob/f1/file4.gz", testBucket), fmt.Sprintf("mc://%v/glob/f3/file5.txt", testBucket), fmt.Sprintf("mc://%v/glob/f5/file-6.txt", testBucket), } for _, pth := range globFiles { if err := createTestFile(pth); err != nil { fmt.Printf("\033[34mSKIP: error creating %s: %v\033[0m\n", pth, err) return } } +defer func() { + for _, pth := range globFiles { + if err := rmTestFile(pth); err != nil { + log.Printf("cleanup: failed to remove object %q: %v", pth, err) + } + } +}() -// run -runRslt := m.Run() +os.Exit(m.Run()) -// remove test objects -for _, pth := range readFiles { - rmTestFile(pth) -} -for _, pth := range globFiles { - rmTestFile(pth) -} - -// remove test bucket -rmBucket(testBucket) - -os.Exit(runRslt) - Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - For external API/resource interactions, only register cleanup after successful initialization, and ensure resources are cleaned up on all exit paths; also handle errors from cleanup calls instead of ignoring them.	Low
More