feat(Improvements): Migrate gcsx integration and random_reader tests to testify - #5031
feat(Improvements): Migrate gcsx integration and random_reader tests to testify#5031charith87 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the integration and random reader tests in internal/gcsx to migrate from the deprecated ogletest framework to standard Go testing using testify/assert and testify/require. This includes introducing helper structs (integrationTestHelper and randomReaderTestHelper) to streamline test setup, teardown, and mocking. The review feedback highlights opportunities to improve test robustness and readability by replacing manual polling loops with require.Eventually and replacing assert.True(reflect.DeepEqual(...)) or assert.True(bytes.Equal(...)) with idiomatic assert.Equal assertions to get better failure diffs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5031 +/- ##
==========================================
- Coverage 83.90% 83.84% -0.07%
==========================================
Files 176 176
Lines 21546 21546
==========================================
- Hits 18078 18065 -13
- Misses 2769 2782 +13
Partials 699 699
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Description
Migrate tests in
internal/gcsxfrom legacyogletesttotestify:integration_test.go: Convert helper and integration tests to usetestify/assertandtestify/require.random_reader_test.go: Migrate random reader unit tests and mock expectations totestify/assert,testify/mock, andtestify/require, and uset.TempDir()for isolated cache directory testing.Helps in standard IDE integration, moving away from unmaintained library, and unifying testing styles.
Link to the issue in case of a bug fix.
N/A
Testing details
make buildpassed.go test -v -count=1 ./internal/gcsx/...passed.Any backward incompatible change? If so, please explain.
N/A