Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove dependecy to testify #190

Merged
merged 4 commits into from
Dec 3, 2019

Conversation

janisz
Copy link
Collaborator

@janisz janisz commented Dec 2, 2019

Fixes: #189

@janisz janisz force-pushed the remove_dependecy_to_testify branch from 6b0058a to ad8f119 Compare December 2, 2019 17:23
bigcache_test.go Outdated
@@ -34,7 +30,7 @@ func TestParallel(t *testing.T) {
go func() {
defer wg.Done()
for i := 0; i < keys; i++ {
sink, _ = cache.Get(fmt.Sprintf("key%d", i))
_, _ = cache.Get(fmt.Sprintf("key%d", i))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cristaloleg I removed sink. Why did we needed this?
BTW: In this test there are no assertion. Is this right?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sink was made to ensure that compiler will not remove operation as no-op.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about deleteing this test? I think it duplicates TestCacheDelRandomly

bigcache/bigcache_test.go

Lines 438 to 498 in ad8f119

// TestCacheDelRandomly does simultaneous deletes, puts and gets, to check for corruption errors.
func TestCacheDelRandomly(t *testing.T) {
t.Parallel()
c := Config{
Shards: 1,
LifeWindow: time.Second,
CleanWindow: 0,
MaxEntriesInWindow: 10,
MaxEntrySize: 10,
Verbose: false,
Hasher: newDefaultHasher(),
HardMaxCacheSize: 1,
Logger: DefaultLogger(),
}
cache, _ := NewBigCache(c)
var wg sync.WaitGroup
var ntest = 800000
wg.Add(1)
go func() {
for i := 0; i < ntest; i++ {
r := uint8(rand.Int())
key := fmt.Sprintf("thekey%d", r)
cache.Delete(key)
}
wg.Done()
}()
wg.Add(1)
go func() {
val := make([]byte, 1024)
for i := 0; i < ntest; i++ {
r := byte(rand.Int())
key := fmt.Sprintf("thekey%d", r)
for j := 0; j < len(val); j++ {
val[j] = r
}
cache.Set(key, val)
}
wg.Done()
}()
wg.Add(1)
go func() {
val := make([]byte, 1024)
for i := 0; i < ntest; i++ {
r := byte(rand.Int())
key := fmt.Sprintf("thekey%d", r)
for j := 0; j < len(val); j++ {
val[j] = r
}
if got, err := cache.Get(key); err == nil && !bytes.Equal(got, val) {
t.Errorf("got %s ->\n %x\n expected:\n %x\n ", key, got, val)
}
}
wg.Done()
}()
wg.Wait()
}

siennathesane
siennathesane previously approved these changes Dec 2, 2019
Copy link
Collaborator

@siennathesane siennathesane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just needs some addressing on @cristaloleg's feedback.

`TestParallel` duplicates `TestCacheDelRandomly`.
@janisz
Copy link
Collaborator Author

janisz commented Dec 3, 2019

@cristaloleg Can you review?

@cristaloleg cristaloleg merged commit b7689f7 into allegro:master Dec 3, 2019
flisky pushed a commit to flisky/bigcache that referenced this pull request May 7, 2020
* Log seed to replicate error

* Remove dependecy to testify

Fixes: allegro#189

* Remove duplicated test

`TestParallel` duplicates `TestCacheDelRandomly`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove dependecy to github.com/stretchr/testify
3 participants