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
Update functional tests to handle NotImplemented error #853
Conversation
functional_tests.go
Outdated
// As few of the features are not available in Gateway(s) currently, Check if err value is NotImplemented, | ||
// and log as NA in that case and continue execution. Otherwise log as failure and return | ||
func validateErrorLog(function string, args map[string]interface{}, startTime time.Time, alert string, message string, err error) { | ||
// If server returns notimplmented we assume it is gateway mode and hence log it as info and move on to next tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
notimplmented - typo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the function could be named as logErrors/logFailures - there is no real validation going on here except to skip logging if it is a NotImplemented error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@poornas @balamurugana can you please review |
functional_tests.go
Outdated
// If server returns NotImplemented we assume it is gateway mode and hence log it as info and move on to next tests | ||
// Special case for ComposeObject API as it is implemented on client side and adds specific error details like `Error in upload-part-copy` in | ||
// addition to NotImplemented error returned from server | ||
if isErrNotImplemented(err) || err.Error() == "Error in upload-part-copy - A header you provided implies functionality that is not implemented" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unable to get why ignoring based on error string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compose-Object
is client side, so it prepends the error string from server with Error in xyz
, (xyz being the exact server api that failed)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On what case we get an error from ComposeObject()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You would need to fix by removing err.Error()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Updated the PR with change in |
For API requests not supported by Minio Gateway, it responds with NotImplemented error. This response needs to be interpreted in functional tests as Not applicable (NA) instead of failure. Error returned by server is now transparently returned by ComposeObject API, so NotImplemented error can be checked by tests Fixes minio/mint#189, minio/mint#194, minio/mint#197
@balamurugana this is updated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
For API requests not supported by Minio Gateway, it responds with
NotImplemented error. This response needs to be interpreted in
functional tests as Not applicable (NA) instead of failure.
Fixes minio/mint#189,
minio/mint#194,
minio/mint#197