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

Fail if directory passed from cmd line doesn't exist #210

Merged
merged 1 commit into from
Nov 5, 2017

Conversation

nitisht
Copy link
Contributor

@nitisht nitisht commented Nov 2, 2017

Mint accepts command line args to run specific tests. This PR adds a check to make sure the test directory exists before attempting to run tests.

mint.sh Outdated
fi
(( i++ ))
done

echo "Finished running all tests."
Copy link
Contributor

Choose a reason for hiding this comment

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

@nitisht , your changes are good - but noticed that we echo "Finished running all tests" even if one of the sdk runs failed. You could check whether your counter i matches the total # of sdks in the run_list and print accurate message of how many sdks passed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the PR @poornas

mint.sh Outdated
echo -n "($i/$count) Running $sdk_name tests ... "
if ! run_test "$sdk_dir"; then
break
if [ -d "$sdk_dir" ]; then
Copy link
Member

@balamurugana balamurugana Nov 3, 2017

Choose a reason for hiding this comment

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

You could do for minimal diff

if [ ! -d "$sdk_dir" ]; then
 echo Test directory "$sdk_name" not found. Exiting Mint.
  exit 1
fi

mint.sh Outdated
fi
(( i++ ))
else
echo Test directory "$sdk_name" not found. Exiting Mint.
Copy link
Member

@balamurugana balamurugana Nov 3, 2017

Choose a reason for hiding this comment

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

Quote the entire string. You should say "Test $sdk_name not found"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

mint.sh Outdated
done

echo "Finished running all tests."
## Report when all tests in run_list are run
if [ $i -eq $((count + 1)) ]; then
Copy link
Member

Choose a reason for hiding this comment

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

IMO you would need to do this as separate PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, will do @balamurugana

@nitisht
Copy link
Contributor Author

nitisht commented Nov 4, 2017

Updated PR @balamurugana

mint.sh Outdated
@@ -125,6 +125,10 @@ function main()
i=1
for sdk_dir in "${run_list[@]}"; do
sdk_name=$(basename "$sdk_dir")
if [ ! -d "$sdk_dir" ]; then
echo "Test directory $sdk_name not found. Exiting Mint."
exit 1
Copy link
Member

Choose a reason for hiding this comment

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

Test directory is not useful to the user. You could print test not found

Copy link
Member

@harshavardhana harshavardhana left a comment

Choose a reason for hiding this comment

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

LGTM tested.

@nitisht nitisht merged commit 7e9333c into minio:master Nov 5, 2017
@nitisht nitisht deleted the non-existing-dir branch November 5, 2017 14:27
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.

4 participants