Skip to content

Commit

Permalink
Migrate downloader and duration tests to testify (#2523)
Browse files Browse the repository at this point in the history 
This allows us to debug/run each test individually which wasn't possible with
ogletest.
  • Loading branch information
@kislaykishore
kislaykishore authored Sep 24, 2024
1 parent 3fa2c49 commit c5cc97f
Show file tree
Hide file tree
Showing 4 changed files with 333 additions and 322 deletions.
82 changes: 46 additions & 36 deletions benchmarks/internal/percentile/duration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,29 +15,20 @@
package percentile_test

import (
"fmt"
"testing"
"time"

"github.com/googlecloudplatform/gcsfuse/v2/benchmarks/internal/percentile"
. "github.com/jacobsa/ogletest"
"github.com/stretchr/testify/assert"
)

func TestDuration(t *testing.T) { RunTests(t) }

////////////////////////////////////////////////////////////////////////
// Boilerplate
////////////////////////////////////////////////////////////////////////

type DurationTest struct {
}

func init() { RegisterTestSuite(&DurationTest{}) }

////////////////////////////////////////////////////////////////////////
// Tests
////////////////////////////////////////////////////////////////////////

func (t *DurationTest) OneObservation() {
func TestOneObservation(t *testing.T) {
t.Parallel()
vals := []time.Duration{
17,
}
Expand All @@ -55,15 +46,18 @@ func (t *DurationTest) OneObservation() {
{100, 17},
}

for _, tc := range testCases {
ExpectEq(
tc.expected,
percentile.Duration(vals, tc.p),
"p: %d", tc.p)
for idx, tc := range testCases {
tc := tc
t.Run(fmt.Sprintf("case_%d", idx), func(t *testing.T) {
t.Parallel()

assert.Equal(t, tc.expected, percentile.Duration(vals, tc.p), "p: %d", tc.p)
})
}
}

func (t *DurationTest) TwoObservations() {
func TestTwoObservations(t *testing.T) {
t.Parallel()
vals := []time.Duration{
100,
200,
Expand All @@ -82,15 +76,21 @@ func (t *DurationTest) TwoObservations() {
{100, 200},
}

for _, tc := range testCases {
ExpectEq(
tc.expected,
percentile.Duration(vals, tc.p),
"p: %d", tc.p)
for idx, tc := range testCases {
tc := tc
t.Run(fmt.Sprintf("case_%d", idx), func(t *testing.T) {
t.Parallel()

assert.Equal(t,
tc.expected,
percentile.Duration(vals, tc.p),
"p: %d", tc.p)
})
}
}

func (t *DurationTest) ThreeObservations() {
func TestThreeObservations(t *testing.T) {
t.Parallel()
vals := []time.Duration{
100,
200,
Expand All @@ -110,15 +110,21 @@ func (t *DurationTest) ThreeObservations() {
{100, 300},
}

for _, tc := range testCases {
ExpectEq(
tc.expected,
percentile.Duration(vals, tc.p),
"p: %d", tc.p)
for idx, tc := range testCases {
tc := tc
t.Run(fmt.Sprintf("case_%d", idx), func(t *testing.T) {
t.Parallel()

assert.Equal(t,
tc.expected,
percentile.Duration(vals, tc.p),
"p: %d", tc.p)
})
}
}

func (t *DurationTest) FiveObservations() {
func TestFiveObservations(t *testing.T) {
t.Parallel()
vals := []time.Duration{
100,
200,
Expand Down Expand Up @@ -157,10 +163,14 @@ func (t *DurationTest) FiveObservations() {
{100, 1000},
}

for _, tc := range testCases {
ExpectEq(
tc.expected,
percentile.Duration(vals, tc.p),
"p: %d", tc.p)
for idx, tc := range testCases {
t.Run(fmt.Sprintf("case_%d", idx), func(t *testing.T) {
t.Parallel()

assert.Equal(t,
tc.expected,
percentile.Duration(vals, tc.p),
"p: %d", tc.p)
})
}
}
89 changes: 46 additions & 43 deletions internal/cache/file/downloader/downloader_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,19 @@ import (
"github.com/googlecloudplatform/gcsfuse/v2/internal/storage/gcs"
testutil "github.com/googlecloudplatform/gcsfuse/v2/internal/util"
"github.com/googlecloudplatform/gcsfuse/v2/tools/integration_tests/util/operations"
. "github.com/jacobsa/ogletest"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
)

var cacheDir = path.Join(os.Getenv("HOME"), "cache/dir")

func TestDownloader(t *testing.T) { RunTests(t) }
func TestDownloaderTestSuite(t *testing.T) {
suite.Run(t, new(downloaderTest))
}

type downloaderTest struct {
suite.Suite
defaultFileCacheConfig *cfg.FileCacheConfig
job *Job
bucket gcs.Bucket
Expand All @@ -49,8 +54,6 @@ type downloaderTest struct {
jm *JobManager
}

func init() { RegisterTestSuite(&downloaderTest{}) }

func (dt *downloaderTest) setupHelper() {
locker.EnableInvariantsCheck()
operations.RemoveDir(cacheDir)
Expand All @@ -64,12 +67,12 @@ func (dt *downloaderTest) setupHelper() {
dt.jm = NewJobManager(dt.cache, util.DefaultFilePerm, util.DefaultDirPerm, cacheDir, DefaultSequentialReadSizeMb, dt.defaultFileCacheConfig)
}

func (dt *downloaderTest) SetUp(*TestInfo) {
func (dt *downloaderTest) SetupTest() {
dt.defaultFileCacheConfig = &cfg.FileCacheConfig{EnableCrc: true}
dt.setupHelper()
}

func (dt *downloaderTest) TearDown() {
func (dt *downloaderTest) TearDownTest() {
dt.fakeStorage.ShutDown()
operations.RemoveDir(cacheDir)
}
Expand All @@ -91,20 +94,20 @@ func (dt *downloaderTest) waitForCrcCheckToBeCompleted() {
func (dt *downloaderTest) verifyJob(job *Job, object *gcs.MinObject, bucket gcs.Bucket, sequentialReadSizeMb int32) {
job.mu.Lock()
defer job.mu.Unlock()
ExpectEq(object.Generation, job.object.Generation)
ExpectEq(object.Name, job.object.Name)
ExpectEq(bucket.Name(), job.bucket.Name())
assert.Equal(dt.T(), object.Generation, job.object.Generation)
assert.Equal(dt.T(), object.Name, job.object.Name)
assert.Equal(dt.T(), bucket.Name(), job.bucket.Name())
downloadPath := util.GetDownloadPath(dt.jm.cacheDir, util.GetObjectPath(bucket.Name(), object.Name))
ExpectEq(downloadPath, job.fileSpec.Path)
ExpectEq(sequentialReadSizeMb, job.sequentialReadSizeMb)
ExpectNe(nil, job.removeJobCallback)
assert.Equal(dt.T(), downloadPath, job.fileSpec.Path)
assert.Equal(dt.T(), sequentialReadSizeMb, job.sequentialReadSizeMb)
assert.NotNil(dt.T(), job.removeJobCallback)
}

func (dt *downloaderTest) Test_CreateJobIfNotExists_NotExisting() {
dt.jm.mu.Lock()
objectPath := util.GetObjectPath(dt.bucket.Name(), dt.object.Name)
_, ok := dt.jm.jobs[objectPath]
AssertFalse(ok)
require.False(dt.T(), ok)
dt.jm.mu.Unlock()

// Call CreateJobIfNotExists for job which doesn't exist.
Expand All @@ -114,8 +117,8 @@ func (dt *downloaderTest) Test_CreateJobIfNotExists_NotExisting() {
defer dt.jm.mu.Unlock()
dt.verifyJob(job, &dt.object, dt.bucket, dt.jm.sequentialReadSizeMb)
actualJob, ok := dt.jm.jobs[objectPath]
AssertTrue(ok)
AssertEq(job, actualJob)
assert.True(dt.T(), ok)
assert.Equal(dt.T(), job, actualJob)
}

func (dt *downloaderTest) Test_CreateJobIfNotExists_Existing() {
Expand All @@ -128,44 +131,44 @@ func (dt *downloaderTest) Test_CreateJobIfNotExists_Existing() {
// Call CreateJobIfNotExists for existing job.
job := dt.jm.CreateJobIfNotExists(&dt.object, dt.bucket)

AssertEq(dt.job, job)
assert.Equal(dt.T(), dt.job, job)
dt.jm.mu.Lock()
defer dt.jm.mu.Unlock()
dt.verifyJob(job, &dt.object, dt.bucket, dt.jm.sequentialReadSizeMb)
actualJob, ok := dt.jm.jobs[objectPath]
AssertTrue(ok)
AssertEq(job, actualJob)
assert.True(dt.T(), ok)
assert.Equal(dt.T(), job, actualJob)
}

func (dt *downloaderTest) Test_CreateJobIfNotExists_NotExisting_WithDefaultFileAndDirPerm() {
dt.jm.mu.Lock()
objectPath := util.GetObjectPath(dt.bucket.Name(), dt.object.Name)
_, ok := dt.jm.jobs[objectPath]
AssertFalse(ok)
assert.False(dt.T(), ok)
dt.jm.mu.Unlock()

// Call CreateJobIfNotExists for job which doesn't exist.
job := dt.jm.CreateJobIfNotExists(&dt.object, dt.bucket)

ExpectEq(0700, job.fileSpec.DirPerm.Perm())
ExpectEq(0600, job.fileSpec.FilePerm.Perm())
assert.EqualValues(dt.T(), 0700, job.fileSpec.DirPerm.Perm())
assert.EqualValues(dt.T(), 0600, job.fileSpec.FilePerm.Perm())
}

func (dt *downloaderTest) Test_GetJob_NotExisting() {
dt.jm.mu.Lock()
objectPath := util.GetObjectPath(dt.bucket.Name(), dt.object.Name)
_, ok := dt.jm.jobs[objectPath]
AssertFalse(ok)
require.False(dt.T(), ok)
dt.jm.mu.Unlock()

// Call GetJob for job which doesn't exist.
job := dt.jm.GetJob(dt.object.Name, dt.bucket.Name())

AssertEq(nil, job)
assert.Nil(dt.T(), job)
dt.jm.mu.Lock()
defer dt.jm.mu.Unlock()
_, ok = dt.jm.jobs[objectPath]
AssertFalse(ok)
assert.False(dt.T(), ok)
}

func (dt *downloaderTest) Test_GetJob_Existing() {
Expand All @@ -178,7 +181,7 @@ func (dt *downloaderTest) Test_GetJob_Existing() {
// Call GetJob for existing job.
job := dt.jm.GetJob(dt.object.Name, dt.bucket.Name())

AssertEq(dt.job, job)
assert.Equal(dt.T(), dt.job, job)
dt.verifyJob(job, &dt.object, dt.bucket, dt.jm.sequentialReadSizeMb)
}

Expand Down Expand Up @@ -206,19 +209,19 @@ func (dt *downloaderTest) Test_GetJob_Concurrent() {
dt.verifyJob(dt.job, &dt.object, dt.bucket, dt.jm.sequentialReadSizeMb)
// Verify all jobs
for i := 0; i < 5; i++ {
ExpectEq(dt.job, jobs[i])
assert.Equal(dt.T(), dt.job, jobs[i])
}
}

func (dt *downloaderTest) Test_InvalidateAndRemoveJob_NotExisting() {
expectedJob := dt.jm.GetJob(dt.object.Name, dt.bucket.Name())
AssertEq(nil, expectedJob)
assert.Nil(dt.T(), expectedJob)

dt.jm.InvalidateAndRemoveJob(dt.object.Name, dt.bucket.Name())

// Verify that job is invalidated and removed from job manager.
expectedJob = dt.jm.GetJob(dt.object.Name, dt.bucket.Name())
AssertEq(nil, expectedJob)
assert.Nil(dt.T(), expectedJob)
}

func (dt *downloaderTest) Test_InvalidateAndRemoveJob_Existing() {
Expand All @@ -229,15 +232,15 @@ func (dt *downloaderTest) Test_InvalidateAndRemoveJob_Existing() {
dt.jm.mu.Unlock()
// Start the job
_, err := expectedJob.Download(context.Background(), 0, false)
AssertEq(nil, err)
assert.Nil(dt.T(), err)

// InvalidateAndRemove the job
dt.jm.InvalidateAndRemoveJob(dt.object.Name, dt.bucket.Name())

// Verify no job existing
AssertEq(Invalid, expectedJob.GetStatus().Name)
assert.Equal(dt.T(), Invalid, expectedJob.GetStatus().Name)
expectedJob = dt.jm.GetJob(dt.object.Name, dt.bucket.Name())
AssertEq(nil, expectedJob)
assert.Nil(dt.T(), expectedJob)
}

func (dt *downloaderTest) Test_InvalidateAndRemoveJob_Concurrent() {
Expand All @@ -248,7 +251,7 @@ func (dt *downloaderTest) Test_InvalidateAndRemoveJob_Concurrent() {
dt.jm.mu.Unlock()
// Start the job
_, err := expectedJob.Download(context.Background(), 0, false)
AssertEq(nil, err)
assert.Nil(dt.T(), err)
wg := sync.WaitGroup{}

// Make concurrent requests
Expand All @@ -263,9 +266,9 @@ func (dt *downloaderTest) Test_InvalidateAndRemoveJob_Concurrent() {
wg.Wait()

// Verify job in invalidated and removed from job manager.
AssertEq(Invalid, expectedJob.GetStatus().Name)
assert.Equal(dt.T(), Invalid, expectedJob.GetStatus().Name)
expectedJob = dt.jm.GetJob(dt.object.Name, dt.bucket.Name())
AssertEq(nil, expectedJob)
assert.Nil(dt.T(), expectedJob)
}

func (dt *downloaderTest) Test_Destroy() {
Expand All @@ -282,32 +285,32 @@ func (dt *downloaderTest) Test_Destroy() {
job2 := dt.jm.CreateJobIfNotExists(&object2, dt.bucket)
// Start the job
_, err := job2.Download(context.Background(), 2, false)
AssertEq(nil, err)
assert.Nil(dt.T(), err)
objectName3 := "path/in/gcs/foo3.txt"
dt.initJobTest(objectName3, objectContent, DefaultSequentialReadSizeMb, uint64(objectSize), func() {})
object3 := dt.object
job3 := dt.jm.CreateJobIfNotExists(&object3, dt.bucket)
// Start the job
_, err = job3.Download(context.Background(), 2, false)
AssertEq(nil, err)
assert.Nil(dt.T(), err)

dt.jm.Destroy()

// Verify all jobs are invalidated
AssertEq(Invalid, job1.GetStatus().Name)
AssertEq(Invalid, job2.GetStatus().Name)
AssertEq(Invalid, job3.GetStatus().Name)
assert.EqualValues(dt.T(), Invalid, job1.GetStatus().Name)
assert.EqualValues(dt.T(), Invalid, job2.GetStatus().Name)
assert.EqualValues(dt.T(), Invalid, job3.GetStatus().Name)
// Verify all jobs are removed
AssertEq(nil, dt.jm.GetJob(objectName1, dt.bucket.Name()))
AssertEq(nil, dt.jm.GetJob(objectName2, dt.bucket.Name()))
AssertEq(nil, dt.jm.GetJob(objectName3, dt.bucket.Name()))
assert.Nil(dt.T(), dt.jm.GetJob(objectName1, dt.bucket.Name()))
assert.Nil(dt.T(), dt.jm.GetJob(objectName2, dt.bucket.Name()))
assert.Nil(dt.T(), dt.jm.GetJob(objectName3, dt.bucket.Name()))
}

func (dt *downloaderTest) Test_CreateJobIfNotExists_InvalidateAndRemoveJob_Concurrent() {
wg := sync.WaitGroup{}
createNewJob := func() {
job := dt.jm.CreateJobIfNotExists(&dt.object, dt.bucket)
AssertNe(nil, job)
assert.NotNil(dt.T(), job)
wg.Done()
}
invalidateJob := func() {
Expand Down
Loading

0 comments on commit c5cc97f

Please sign in to comment.