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

Tox support for cross-versions testing #960

Merged
merged 12 commits into from Apr 29, 2015
Merged

Tox support for cross-versions testing #960

merged 12 commits into from Apr 29, 2015

Conversation

noirbizarre
Copy link
Collaborator

Mongoengine is tested on different PyMongo versions, different Python versions...
This pull-request add a tox.ini file allowing to run the test suite on every supported version composition .
This allow to do proper testing before submitting pull-requests without having to wait for TravisCI cross-versions testing.

To run the test in tox, simply run:

$ tox

It will create a virtualenv for each combination and run the same test suite (like TravisCI).

Review on Reviewable

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling 8d291b7 on noirbizarre:tox into 19dc312 on MongoEngine:master.

@noirbizarre noirbizarre changed the title Tox Tox support for cross-versions testing Apr 16, 2015
@MRigal
Copy link
Member

MRigal commented Apr 16, 2015

Yep 👍

@thedrow
Copy link
Contributor

thedrow commented Apr 16, 2015

Can you please add Django 1.8 as well?

@thedrow
Copy link
Contributor

thedrow commented Apr 16, 2015

Maybe we should run the tests using tox in Travis as well?

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling bc395df on noirbizarre:tox into 19dc312 on MongoEngine:master.

@noirbizarre
Copy link
Collaborator Author

I will look into it but last time I saw some project matching travis builds to tox builds it was a little bit complex.

Maybe I should send another pull-request later allowing to merge and benefit from tox right now.

@thedrow
Copy link
Contributor

thedrow commented Apr 16, 2015

LGTM.
Let's see what the other contributors think.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 73.03% when pulling 8d291b7 on noirbizarre:tox into 19dc312 on MongoEngine:master.

6 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 73.03% when pulling 8d291b7 on noirbizarre:tox into 19dc312 on MongoEngine:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 73.03% when pulling 8d291b7 on noirbizarre:tox into 19dc312 on MongoEngine:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 73.03% when pulling 8d291b7 on noirbizarre:tox into 19dc312 on MongoEngine:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 73.03% when pulling 8d291b7 on noirbizarre:tox into 19dc312 on MongoEngine:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 73.03% when pulling 8d291b7 on noirbizarre:tox into 19dc312 on MongoEngine:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 73.03% when pulling 8d291b7 on noirbizarre:tox into 19dc312 on MongoEngine:master.

@rozza
Copy link
Contributor

rozza commented Apr 16, 2015

tox ftw 👍

@rozza
Copy link
Contributor

rozza commented Apr 16, 2015

Might be helpful for travis: https://www.dominicrodger.com/tox-and-travis.html

@noirbizarre
Copy link
Collaborator Author

Yes, I have a start of Tox/Travis coupling implementation but some features will be lost:

  • coverage will occur only once in a specific job (no more by job coverage on coveralls.io)
  • benchmark will be lost

If everyone is ok with these change, I will submit this as new pull-request for Travis coupling as soon as this one is merged (I don't want to break TravisCI build in this one)

@thedrow
Copy link
Contributor

thedrow commented Apr 16, 2015

You can run the benchmarks on a different tox job regardless of the tests.

@thedrow
Copy link
Contributor

thedrow commented Apr 16, 2015

Also, you can run all tox jobs with coverage.

@noirbizarre
Copy link
Collaborator Author

Yep, forcing coverage on all jobs this is the alternative.
Local testing will be a bit slower, but not very much.

About benchmarks, I can create a tox job for this, but I don't think TravisCI is the place to run them.
If the build reach them, they won't fail ever.
The information provided by the benchmark is not build related (OK/KO)

@MRigal
Copy link
Member

MRigal commented Apr 16, 2015

If you're already touching a bit the Travis configuration, you may edit it to remove the benchmark from the pypy3 tests, since they are now the only reason for failure under pypy3, like in the output below

python benchmark.py

Benchmarking...

Creating 10000 dictionaries - Pymongo

5.754539966583252

Creating 10000 dictionaries - Pymongo write_concern={"w": 0}

4.454897165298462

Creating 10000 dictionaries - MongoEngine
Traceback (most recent call last):
File "benchmark.py", line 283, in
main()
File "benchmark.py", line 197, in main
print(t.timeit(1))
File "/opt/python/pypy3-2.4.0/lib-python/3/timeit.py", line 195, in timeit
timing = self.inner(it, self.timer)
File "", line 9, in inner
File "/home/travis/build/MongoEngine/mongoengine/mongoengine/init.py", line 1, in
import document
ImportError: No module named document

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 73.01% when pulling bc395df on noirbizarre:tox into 19dc312 on MongoEngine:master.

6 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 73.01% when pulling bc395df on noirbizarre:tox into 19dc312 on MongoEngine:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 73.01% when pulling bc395df on noirbizarre:tox into 19dc312 on MongoEngine:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 73.01% when pulling bc395df on noirbizarre:tox into 19dc312 on MongoEngine:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 73.01% when pulling bc395df on noirbizarre:tox into 19dc312 on MongoEngine:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 73.01% when pulling bc395df on noirbizarre:tox into 19dc312 on MongoEngine:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 73.01% when pulling bc395df on noirbizarre:tox into 19dc312 on MongoEngine:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+15.29%) to 88.32% when pulling 220e1d7 on noirbizarre:tox into 7e1a5ce on MongoEngine:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+15.29%) to 88.32% when pulling 220e1d7 on noirbizarre:tox into 7e1a5ce on MongoEngine:master.

@mmellison
Copy link
Contributor

@noirbizarre You should remove Django from the matrix and think about adding in PyMongo==dev instead. Then it looks like this should be ready to go once #973 is merged.

@noirbizarre
Copy link
Collaborator Author

Yep, once #973 and #946 are merged, I will:

  • drop Django form the Matrix
  • add PyMongo 3 and PyMongo dev

@mmellison
Copy link
Contributor

@noirbizarre It looks like #946 might take a bit more time, as it sounds like @MRigal (get well soon!) is going to be unavailable for some time and there are still some outstanding issues.

How about we remove Django from the matrix and commit? We can add in newer pymongo support in the matrix as part of #946 once it gets closer to acceptance.

@noirbizarre
Copy link
Collaborator Author

Done:
I rebased the branch, removed Django from tests, removed some Django reference from the README.
I also prepared for PyMongo 3 and PyMongo dev, you will just have to uncomment the line (and remove the old ones if any) from .travis.yml and tox.ini.

@mmellison
Copy link
Contributor

Awesome! We really appreciate your work on this. It will go a long way in helping with test coverage and code quality assurance!

I will merge in the PR once Travis-CI gets caught up.

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling c804c39 on noirbizarre:tox into dfc9dc7 on MongoEngine:master.

@noirbizarre
Copy link
Collaborator Author

I find tox be one of the more helpful tool when providing cross-version compatibility on a library so it's often one of the first pull-request I do.
It helps so much not having to wait for TravisCI (which is longuer and longuer) for testing the next pull-request !

mmellison added a commit that referenced this pull request Apr 29, 2015
Tox support for cross-versions testing
@mmellison mmellison merged commit 42ba4a5 into MongoEngine:master Apr 29, 2015
@coveralls
Copy link

Coverage Status

Coverage increased (+17.11%) to 91.69% when pulling c804c39 on noirbizarre:tox into dfc9dc7 on MongoEngine:master.

noirbizarre added a commit to noirbizarre/flask-mongoengine that referenced this pull request May 13, 2015
@noirbizarre noirbizarre deleted the tox branch May 13, 2015 16:54
ghost pushed a commit to MongoEngine/flask-mongoengine that referenced this pull request May 15, 2015
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.

None yet

7 participants