Add a test module against a real running server #131

gst · 2015-03-23T21:43:01Z

This PR brings basically 2 things:

Add a "client_test_with_server.py" test module, that will upon execution, launch a fresh influxdb server instance in a temporary place.
Correct few detected bad things in the client code itself.

any comments ?

client_test_with_server.py could now easily be extended with others (more complex) test cases..

aviau · 2015-03-24T00:03:31Z

tests/influxdb/real_client_test.py

+            shutil.rmtree(self.influxd_temp_dir_base)
+
+
+class TestClientWithNoServer(unittest.TestCase):


Those tests already exist.

effectively, had done a copy of client_test.py to start this one.. ;)
how about making client_test.py renamed to client_tests_without_server.py, and this one renamed to client_tests_with_server.py and also effectively remove this from here ?
or just rename this one to client_tests_with_server.py to insist enough ?

I would prefer if you renamed the new file to client_tests_with_server.py and keep the current one as it is.

aviau · 2015-03-24T00:17:06Z

Very good job @gst :) This is awesome.

I wouldn't build from source. The lib tries to target the last InfluxDB release, not master. This would also take out some of the complexity of your PR. I would prefer if you just wget a .deb and install it. We could change the link whenever we want to target a new release.
The tests don't work on Travis yet
There is still only two flake8 errors.

gst · 2015-03-24T00:59:43Z

don't merge this already so, I'll also do a slight change in real_client_test (among its probable rename).
g.

gst · 2015-03-24T01:20:35Z

I wouldn't build from source.

I wanted to be sure to also have some tests against the latest master server version here, so that's the reason.

The lib tries to target the last InfluxDB release, not master.

Right, but this greatly helps to keep up to date with influxdb server master so, that's what I simply wanted to get at the initial stage in fact, and I've added this to the PR so to have it right there for the (near) future ;)

This would also take out some of the complexity of your PR.

relative complexity ;) that's just an install of ruby and few others packages, a gem install and a build & deb install finally, and within the tests a creation and start of a fresh server instance also. Advanced but not complex :p

I would prefer if you just wget a .deb and install it. We could change the link whenever we want to target a new release.

That's the other possibility I've also think about. Not sure it's better..

gst · 2015-03-24T01:27:25Z

I have also missed that the deb fails to install damn.. will check that also..

aviau · 2015-03-24T23:22:26Z

.travis.yml

@@ -12,6 +12,7 @@ env:
 install:
    - sudo pip install tox
    - sudo pip install coveralls
+    - ./build_influxdb_server.sh || true


Why the || true ?

Cause wasn t sure of need of command to return exit code of true (0) to continue test run..

Can you remove it?

aviau · 2015-03-24T23:25:40Z

The tests now work on Travis :)

aviau · 2015-03-24T23:29:17Z

Next time, please don't put unrelated changes in the same PR.

The ''Random python fixes" should not be in the same PR as the integration tests.

gst · 2015-03-24T23:34:08Z

You would make separate pr for each "pythonic" change ? (There was about 3 hère)

aviau · 2015-03-24T23:37:25Z

Yes

If its not too much trouble for you, one PR for each change is best, but one PR for the integration tests and one pr for "pythonic changes" would have been fine. Don't worry about it for this PR tho.

gst · 2015-03-25T13:54:07Z

Yes

ok then ;)

I'm splitting this one in 2 PR : this one but "cleaned" ; and one for each "pythonic" change unrelated to this test_client_with_server.py and I'll rebase this one without thoses changes so.

stay tuned..

gst · 2015-03-25T14:23:29Z

@ReAzem : if you are ok to merge the "pythonic" PR (#133) first ?
I'll then rebase this one on it. (hmm, maybe i can already do it in fact, gonna try..)

aviau · 2015-03-25T14:27:07Z

I'll be merging #133 in a few mins when the tests are done.

aviau · 2015-03-25T14:47:37Z

You should be able to rebase now.

By *real* we mean : one which really connect to a server instance.

So to be able to run client_test_with_server.py.

gst · 2015-03-25T14:55:23Z

rebased,
should just pass.. (if i've not forgotten some thing(s))

aviau · 2015-03-25T14:55:30Z

Oh yeah! Thats a clean PR ;)

gst · 2015-03-25T14:56:45Z

that's better/cleaner effectively ;)
don't know why but the list of commits here appears in another order than in the branch :?

aviau · 2015-03-25T15:01:04Z

don't know why but the list of commits here appears in another order than in the branch :?

don't worry about it.

For some reason GitHub history can be pretty hard to read. I still don't know if they list commits in "author" or in "apply" order.

@gst

Add a test module against a real running server (Thanks @gst!)

aviau · 2015-03-25T18:18:12Z

Thank you! :)

aviau reviewed Mar 24, 2015
View reviewed changes

gst force-pushed the build_and_install_server branch from 715f8e4 to 9ae38d3 Compare March 25, 2015 14:28

Grégory Starck added 4 commits March 25, 2015 10:53

Add: implemented a "real" client test module.

48e08a1

By *real* we mean : one which really connect to a server instance.

Add: build server package and install it.

d86ad1f

So to be able to run client_test_with_server.py.

For python2/3 compat.

e8ea370

Fix: query() requires database argument.

b00409a

gst force-pushed the build_and_install_server branch from 9ae38d3 to b00409a Compare March 25, 2015 14:54

aviau added a commit that referenced this pull request Mar 25, 2015

Merge pull request #131 from savoirfairelinux/build_and_install_server

a3c383e

Add a test module against a real running server (Thanks @gst!)

aviau merged commit a3c383e into influxdata:master Mar 25, 2015

aviau deleted the build_and_install_server branch March 25, 2015 18:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a test module against a real running server #131

Add a test module against a real running server #131

gst commented Mar 23, 2015

aviau Mar 24, 2015

gst Mar 24, 2015

aviau Mar 24, 2015

aviau commented Mar 24, 2015

gst commented Mar 24, 2015

gst commented Mar 24, 2015

gst commented Mar 24, 2015

aviau Mar 24, 2015

gst Mar 24, 2015

aviau Mar 24, 2015

aviau commented Mar 24, 2015

aviau commented Mar 24, 2015

gst commented Mar 24, 2015

aviau commented Mar 24, 2015

gst commented Mar 25, 2015

gst commented Mar 25, 2015

aviau commented Mar 25, 2015

aviau commented Mar 25, 2015

gst commented Mar 25, 2015

aviau commented Mar 25, 2015

gst commented Mar 25, 2015

aviau commented Mar 25, 2015

aviau commented Mar 25, 2015

		shutil.rmtree(self.influxd_temp_dir_base)


		class TestClientWithNoServer(unittest.TestCase):

Add a test module against a real running server #131

Add a test module against a real running server #131

Conversation

gst commented Mar 23, 2015

aviau Mar 24, 2015

Choose a reason for hiding this comment

gst Mar 24, 2015

Choose a reason for hiding this comment

aviau Mar 24, 2015

Choose a reason for hiding this comment

aviau commented Mar 24, 2015

gst commented Mar 24, 2015

gst commented Mar 24, 2015

gst commented Mar 24, 2015

aviau Mar 24, 2015

Choose a reason for hiding this comment

gst Mar 24, 2015

Choose a reason for hiding this comment

aviau Mar 24, 2015

Choose a reason for hiding this comment

aviau commented Mar 24, 2015

aviau commented Mar 24, 2015

gst commented Mar 24, 2015

aviau commented Mar 24, 2015

gst commented Mar 25, 2015

gst commented Mar 25, 2015

aviau commented Mar 25, 2015

aviau commented Mar 25, 2015

gst commented Mar 25, 2015

aviau commented Mar 25, 2015

gst commented Mar 25, 2015

aviau commented Mar 25, 2015

aviau commented Mar 25, 2015