-
Notifications
You must be signed in to change notification settings - Fork 19
Large tests and runnable example #20
Large tests and runnable example #20
Conversation
marcin-krolik
commented
Sep 15, 2016
- runnable example added
- large tests implemented in python
PR #19 has to be merged before this one in order to enable downloading plugin binary from proper path at S3 |
570b5a0
to
87c0bd1
Compare
mkdir -p $PLUGIN_PATH | ||
|
||
_info "downloading plugins" | ||
(cd $PLUGIN_PATH && curl -sSO http://snap.ci.snap-telemetry.io/snap/master/latest/snap-plugin-publisher-mock-file && chmod 755 snap-plugin-publisher-mock-file) |
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 f
and L
flags for handling errors in the curl. ... curl -sfLSO ...
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.
Fixed
|
||
_info "downloading plugins" | ||
(cd $PLUGIN_PATH && curl -sSO http://snap.ci.snap-telemetry.io/snap/master/latest/snap-plugin-publisher-mock-file && chmod 755 snap-plugin-publisher-mock-file) | ||
(cd $PLUGIN_PATH && curl -sSO http://snap.ci.snap-telemetry.io/plugins/snap-plugin-collector-meminfo/latest/linux/x86_64/snap-plugin-collector-meminfo && chmod 755 snap-plugin-collector-meminfo) |
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.
The URL will change to http://snap.ci.snap-telemetry.io/plugins/snap-plugin-collector-meminfo/latest_build/linux/x86_64/snap-plugin-collector-meminfo
. #22
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.
Goo catch, missed it somehow
When I run the test I got an error. Here is the full log:
Example is working. |
@kindermoumoute this was due to changes introduces in snap framework, specific to Nevertheless test will fail with latest snap build. Reason behind it is this bug |
5ad0de6
to
2ce48e8
Compare
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.
The goal of doing those large tests is to make it run on K8S. For that you need to add a deployment file, and large_k8s.sh
that will run by running TEST_K8S=1 make test-large
.
Example:
https://github.com/intelsdi-x/snap-plugin-publisher-influxdb/blob/master/scripts/config/influxdb-deployment.yml
https://github.com/intelsdi-x/snap-plugin-publisher-influxdb/blob/master/scripts/large_k8s.sh
@kindermoumoute I don't think it's needed to add |
124da7c
to
9f9abd7
Compare
9f9abd7
to
317cad8
Compare
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.
When I run test for K8S, there is a permission denied
error:
❯ TEST_K8S=1 make test-large
bash -c "./scripts/test.sh large"
2016-10-03 17:16:57 UTC [ info] updating repository submodule with pytest
2016-10-03 17:16:57 UTC [ info] execute large test
2016-10-03 10:16:57 [ INFO ] Downloading http://snap.ci.snap-telemetry.io/snap/master/latest/snapd to /usr/local/bin
2016-10-03 10:17:06 [ INFO ] Downloading http://snap.ci.snap-telemetry.io/snap/master/latest/snapctl to /usr/local/bin
2016-10-03 10:17:07 [ INFO ] Downloading http://snap.ci.snap-telemetry.io/plugins/snap-plugin-collector-meminfo/latest_build/linux/x86_64/snap-plugin-collector-meminfo to /etc/snap/plugins
2016-10-03 10:17:07 [ ERROR ] 13
E
======================================================================
ERROR: test_meminfo_collector_plugin (__main__.MemInfoCollectorLargeTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/ocano/work/src/github.com/intelsdi-x/snap-plugin-collector-meminfo/scripts/test/large.py", line 45, in setUp
utils.download_binaries(self.binaries)
File "/Users/ocano/work/src/github.com/intelsdi-x/snap-plugin-collector-meminfo/scripts/test/pytest/utils.py", line 28, in download_binaries
_download_binary(binary)
File "/Users/ocano/work/src/github.com/intelsdi-x/snap-plugin-collector-meminfo/scripts/test/pytest/utils.py", line 45, in _download_binary
_ensure_dir(binary.dir)
File "/Users/ocano/work/src/github.com/intelsdi-x/snap-plugin-collector-meminfo/scripts/test/pytest/utils.py", line 33, in _ensure_dir
os.makedirs(dirname)
File "/usr/local/Cellar/python/2.7.12/Frameworks/Python.framework/Versions/2.7/lib/python2.7/os.py", line 150, in makedirs
makedirs(head, mode)
File "/usr/local/Cellar/python/2.7.12/Frameworks/Python.framework/Versions/2.7/lib/python2.7/os.py", line 157, in makedirs
mkdir(name, mode)
OSError: [Errno 13] Permission denied: '/etc/snap'
----------------------------------------------------------------------
Ran 1 test in 9.879s
FAILED (errors=1)
More a general comment, using a python framework here makes it harder for developer to understand plugin codebase:
Which bring us to ask those question:
This would bring us to have an agreement on what we want to test for large test. This PR is testing more cases than other existing large tests, which is really cool.
I would be interested to have your thoughts on those questions. Also, you can take a look on a proposal I made here to have the current test framework synchronized with pluginsync. This is one solution, I believe there are other solutions. |
LGTM |
If we plan to assure quality and stability of plugins and framework I think we need a framework for such kind of tests. It's going to introduce new code base and need to maintain it. I don't know of any other way to achieve reasonable quality of whole product.
Sure. What I already prepared is a example of such framework.
I think it more of a question "When plugin is complete?". Is it enough to provide unit tests and documentation? Or maybe runnable example as well? How about large tests? |
@kindermoumoute As for your comment regarding |
LGTM |