-
Notifications
You must be signed in to change notification settings - Fork 50
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
Add s3cmd tests. #182
Add s3cmd tests. #182
Conversation
build/s3cmd/install.sh
Outdated
exit 1 | ||
fi | ||
|
||
pip3 install s3cmd==$S3CMD_VERSION |
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.
As package version is optional to pip, you can install latest s3cmd by pip3 install s3cmd
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.
Sure.
@@ -0,0 +1,2 @@ | |||
*~ | |||
*.log |
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.
add new line
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.
Surw
run/core/s3cmd/test.sh
Outdated
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
# | ||
|
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.
Can you style/logic of https://github.com/minio/mc/blob/master/functional-tests.sh?
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.
Why .. I based this on awscli?
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.
current awscli code is bug prone, unclean and not simple. I suggested the team to clean awscli too.
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.
Makes sense..
Tests updated @balamurugana @nitisht to be more |
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 & Tested. One comment regarding logs:
As per our last discussion, logs should be formatted like this
{
"name": "s3cmd/test_make_bucket",
"duration": "424",
"function": "mb",
"args": {
bucketName: ""
}
"status": "PASS"
}
and so on. We can either fix it here or take it up in next pod.
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.
minor comment
run/core/s3cmd/test.sh
Outdated
log_success "$start_time" "${FUNCNAME[0]}" | ||
} | ||
|
||
function test_cp_object() |
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 can name this as test_copy_object()
for more readability.
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
No description provided.