Skip to content

Clipper Manager Python Tests#187

Merged
dcrankshaw merged 37 commits intoucbrise:developfrom
Corey-Zumar:python_tests
Jun 3, 2017
Merged

Clipper Manager Python Tests#187
dcrankshaw merged 37 commits intoucbrise:developfrom
Corey-Zumar:python_tests

Conversation

@Corey-Zumar
Copy link
Contributor

Test via:

# Execute short tests (total execution time < 1 min)
$ python /management/test/clipper_manager_test.py short
# Or execute long tests (total execution time ~ 10 mins)
$ python /management/test/clipper_manager_test.py long
# Or execute all tests
$ python /management/test/clipper_manager_test.py

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/340/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/341/
Test FAILed.

Copy link
Contributor

@dcrankshaw dcrankshaw left a comment

Choose a reason for hiding this comment

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

This is great! It will be really good to have test coverage of clipper_manager as we start refactoring it.

echo -e "\nRunning management tests\n\n"
./src/management/managementtests --redis_port $REDIS_PORT
cd $DIR
python ../management/test/clipper_manager_test.py
Copy link
Contributor

Choose a reason for hiding this comment

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

clipper_manager.py got moved from the management/ to the clipper_admin/ directory as part of making the pip package. Move the tests to the same place (clipper_admin/test).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


if __name__ == '__main__':
run_short = True
run_long = True
Copy link
Contributor

Choose a reason for hiding this comment

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

There should be a run_all option as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

self.assertGreaterEqual(len(running_containers_output), 1)


class ClipperManagerTestCaseLong(unittest.TestCase):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you import sklearn here so that the short test case can still be run without sklearn installed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both cases requiresklearn. It is required in test_model_deploys_successfully (short case) and test_deployed_model_queried_successfully (long case).

cur_dir = os.path.dirname(os.path.abspath(__file__))
sys.path.insert(0, os.path.abspath('%s/../' % cur_dir))
import clipper_manager

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a comment at the top of the file briefly describing how these tests are run? Basically just say that at the beginning of each test suite a local Clipper instance is created and the tests issue various commands against the running Clipper instance and verify their results.

This is just so that it's documented that these tests start Docker containers and interact with them, as opposed to being pure Python tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@Corey-Zumar
Copy link
Contributor Author

@dcrankshaw Addressed your comments.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/366/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/368/
Test FAILed.

Copy link
Contributor

@dcrankshaw dcrankshaw left a comment

Choose a reason for hiding this comment

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

I pushed a couple really minor cleanup changes.

I commented out the deploy_predict_function tests until #194 gets merged because dealing with conda environments in jenkins is a little messy and I don't want to be blocked on that.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/369/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/370/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/373/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/374/
Test FAILed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Clipper-PRB/375/
Test PASSed.

@dcrankshaw dcrankshaw merged commit bfa82b9 into ucbrise:develop Jun 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants