-
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
Improved logging and tests for mc #105
Conversation
@CarterMcClellan looks like |
Also, here a document of all tests which I am working towards implementing in mint. Doc has been updated to reflect updated tests, and removal of event related tests from the TODO agenda |
@CarterMcClellan I think it will be better to add the doc as |
run/core/mc/run.sh
Outdated
@@ -43,9 +43,10 @@ main() { | |||
setupMCTarget >>"$logfile" 2>&1 || { echo 'mc setup failed' ; exit 1; } | |||
|
|||
run 2>>"$errfile" 1>>"$logfile" || { echo 'mc run failed.'; rc=1; } | |||
grep -e '<ERROR>' "$logfile" >> "$errfile" | |||
# grep -e '<ERROR>' "$logfile" >> "$errfile" |
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.
It will be better to remove the old echo
statements instead of commenting them out, for all the occurences..
run/core/mc/test.sh
Outdated
if [ "$(basename "$(./mc cp --json "target/${bucketName}/datafile-65-MB" /tmp | jq -r .target)")" != "datafile-65-MB" ]; then | ||
>&2 echo "Put Object Multipart Test 1 Failure" | ||
>&2 echo "Line Number: 105" |
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.
what are these static line numbers for? If it is to indicate failure point, I think it is better to remove these as line numbers will change in due course and it will be difficult to maintain.
run/core/mc/test.sh
Outdated
then | ||
printf "\tList Objects Test 1 Failure\n" | ||
>&2 echo "List Objects Test 1 Failure" | ||
>&2 echo "Line Number 263" |
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.
Looks like spacing issue here..
run/core/mc/test.sh
Outdated
printf "\tCat Objects Test 1 Failure\n" | ||
>&2 echo "Cat Objects Test 1 Failure" | ||
>&2 echo "Line 291" | ||
fi |
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.
spacing issue..
run/core/mc/test.sh
Outdated
watchObjects(){ | ||
local bucketName | ||
bucketName=$(create_random_string) | ||
./mc mb play/"$bucketName" >/dev/null |
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.
Lots of tabbing issues..
run/core/mc/test.sh
Outdated
remove_bucket "${bucketName}" | ||
printf "\tTest Success\n" | ||
} | ||
|
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 comments for each 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.
done
@CarterMcClellan I tried running these changes locally
Looks like the logs are not properly reported (see |
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 use a shell mode in your editor and fix these tabbing problems?
run/core/mc/test.sh
Outdated
local bucketName | ||
bucketName=$(create_random_string) | ||
|
||
./mc mb "target/$bucketName" >/dev/null |
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.
I think there is some tabbing problem still in these scripts.
@CarterMcClellan i added a comment please take a look. |
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.
Prior to the changes mc printed every step to output.log, and logged the tests
which were intended to throw errors under error.log.
mc output.log will now provide colorized print of all tests which are executed,
all progress bars and extraneous prints have been removed to make output.log
more readable. mc error.log will only hold errors if an unintentional error is
thrown, the convention followed for logging is that the test which failed will
be printed to both output and error.log, inside of error.log a line number which
the test failed on will also be provided. Because these tests are being done
on mc error logging can be improved with the --debug flag. For mc share
upload, and mc share download (the more complicated commands), --debug
stack traces are logged under error.log if an error is found.
In accordance with the doc tests for watch (experimental), cat, and ls have
all been added to mc.
Fixes #86