tests: Reduce context timeout to avoid false positives #881
Conversation
|
functional test on Travis failed for the following {"alert":"","args":{"bucketName":"minio-go-testxns0okpetpou4rs6k","ctx":{"Context":0},"objectName":"0n1i3humrguu2dnrzrtgg0vaf49g3z"},"duration":156,"function":"GetObjectWithContext(ctx, bucketName, objectName)","message":"GetObjectWithContext failed due to non-cancellation upon short timeout","name":"minio-go","status":"fail"} |
harshavardhana
force-pushed
the
final-fix
branch
from
b618e0d
to
92c1f9a
Compare
|
The PR is updated and fixed the context usage properly. |
functional_tests.go
Outdated
| return | ||
| } | ||
| if _, err = r.Stat(); err == nil { | ||
| logError(function, args, startTime, "", "GetObjectWithContext did not failue to due to cancellation upon short timeout", err) |
|
Any other concerns other than the typo ? |
harshavardhana
force-pushed
the
final-fix
branch
from
92c1f9a
to
7848c69
Compare
functional_tests.go
Outdated
| args["ctx"] = ctx | ||
| args["opts"] = minio.PutObjectOptions{ContentType: "binary/octet-stream"} | ||
| defer cancel() | ||
|
|
||
| _, err = c.PutObjectWithContext(ctx, bucketName, objectName, reader, int64(bufSize), minio.PutObjectOptions{ContentType: "binary/octet-stream"}) | ||
| if err != nil { | ||
| if err == nil { |
There was a problem hiding this comment.
Looks like this has been changed to expect the API to fail. If that is the case, the log message needs to reflect that.
functional_tests.go
Outdated
| return | ||
| } | ||
| ctx, cancel = context.WithTimeout(context.Background(), 3*time.Minute) | ||
| if _, err = r.Stat(); err == nil { | ||
| logError(function, args, startTime, "", "GetObjectWithContext did not failure to due to cancellation upon short timeout", err) |
There was a problem hiding this comment.
typo: failure needs to be fail
functional_tests.go
Outdated
| return | ||
| } | ||
| if _, err = r.Stat(); err == nil { | ||
| logError(function, args, startTime, "", "GetObjectWithContext did not failure to due to cancellation upon short timeout", err) |
There was a problem hiding this comment.
The comment needs to be clear
context timeout tests had long timeouts leading to false positives in some cases when functional tests are run against faster disks. Reduce them such that the possibility of this would never occur in future usage. Fixes minio#880
harshavardhana
force-pushed
the
final-fix
branch
from
7848c69
to
a289133
Compare
|
fixed comments PTAL @kannappanr @poornas |
context timeout tests had long timeouts leading
to false positives in some cases when functional
tests are run against faster disks.
Reduce them such that the possibility of this
would never occur in future usage.
Fixes #880