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

Run tests with both python2 and python3 #535

Merged
merged 4 commits into from
Jun 27, 2017

Conversation

poornas
Copy link
Contributor

@poornas poornas commented Jun 15, 2017

Fixes #532

This is just a convenience script for developers to ensure that unit tests and functional tests pass with both python2 and python3

Commit 2c2046e is identical to fix by @vadmeste for #533

@harshavardhana
Copy link
Member

@poornas @vadmeste has already submitted a previous change for this.. Can you guys work together and merge the PRs? or perhaps anything that is not associated with tests compatibility with python3 be taken out and submitted with a different PR?

@@ -52,7 +52,6 @@ def main():
# client.trace_on(sys.stderr)

# Make a new bucket.
bucket_name = 'minio-pytest'
Copy link
Member

Choose a reason for hiding this comment

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

Is it safe to remove this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

line 48: bucket_name = uuid.uuid4().str() will generate a unique bucket name. Otherwise the make_bucket test will fail with hardcoded bucket name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@harshavardhana , I will rebase after #534 is merged to master.

@poornas poornas force-pushed the issue/532 branch 3 times, most recently from a76fb38 to 394bc44 Compare June 17, 2017 00:01
@harshavardhana
Copy link
Member

Can you rebase this @poornas ?

@poornas
Copy link
Contributor Author

poornas commented Jun 22, 2017

@harshavardhana , rebased..

run() {
echo "running..."
if [[ $py2flag = 1 ]]; then
python ./functional/tests.py
Copy link
Member

Choose a reason for hiding this comment

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

Some tabbing problem.

@@ -0,0 +1,64 @@
#!/usr/bin/env bash
#!/usr/bin/expect -f
Copy link
Member

Choose a reason for hiding this comment

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

why is expect used here?

#!/usr/bin/env bash
#!/usr/bin/expect -f
#
# Minio Cloud Storage, (C) 2017 Minio, Inc.
Copy link
Member

Choose a reason for hiding this comment

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

Have python licensing heading here.

#

#Make sure following packages are installed on your computer:
#pip install nosetests
Copy link
Member

Choose a reason for hiding this comment

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

Comments should have #<space> and also why are these written here?

harshavardhana
harshavardhana previously approved these changes Jun 26, 2017
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.

@vadmeste waiting on your review.


# invoke the script

main "$@"
Copy link
Member

Choose a reason for hiding this comment

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

@poornas, if you start the script from any directory (minio-py for example), you will get an error. The solution is to make the script to change the current directory to where the script lives and then invoke the command.

The diff below will fix the problem:

diff --git a/tests/functional_test.sh b/tests/functional_test.sh
index 06b1581f..32711571 100755
--- a/tests/functional_test.sh
+++ b/tests/functional_test.sh
@@ -58,6 +58,6 @@ main () {
 
 }
 
-# invoke the script
-
-main "$@"
+# Move to the directory which contains this script and invoke it
+cd $(dirname $(realpath $0)) ; \
+               main "$@"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vadmeste, done

Copy link
Member

@vadmeste vadmeste left a comment

Choose a reason for hiding this comment

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

One minor change needed

@poornas
Copy link
Contributor Author

poornas commented Jun 27, 2017

@vadmeste, your comments were addressed

Copy link
Member

@vadmeste vadmeste left a comment

Choose a reason for hiding this comment

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

LGTM and tested

@harshavardhana harshavardhana merged commit bf4a611 into minio:master Jun 27, 2017
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.

tests: Check for both python 2 and 3
3 participants