Skip to content

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 👍

@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.

@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)

@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 remained the same at 73.03% when pulling bc395df on noirbizarre:tox into 19dc312 on MongoEngine:master.

@noirbizarre
Copy link
Collaborator Author

OK, I think I may have come to a very clean solution with the last commit:

  • use python setup.py nosetests instead of python setup.py test (as officialy recommanded in the documentation)
    • allow to use plugins
    • allow to pass options to nose
  • added rednose plugin for colored output
  • cleaned the setup.cfg for proper default configuration (mostly rednose and coverage)
  • tight coupling between TravisCI builds and tox
    • by jobs coverage (but still optionnal)
    • disabled benchmark.py (should be run manually, it's an indicator) (a tox task can be added if wanted)
  • updated documentations on testing
  • re-allow failure on Pypy3 (as benchmark is removed from build)

To pass options to tox, you have to add them after --.:

# Run tests
$ python setup.py nosetests
# Run tests with coverage
$ python setup.py nosetests --with-coverage
# Run all versions combinations
$ tox
# Run all versions combinations with coverage
$ tox -- --with coverage

The build matric is pretty big right now with Django (cf. #958 ), it generate 96 different builds so I have not enabled testing on PyMongo==dev right now.

I've noticed some other possible optimization and bug fixes but I will submit them as a separate pull-requests.

What do you think ?

@landscape-bot
Copy link

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

@landscape-bot
Copy link

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

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling 5031b29 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.

6 participants