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

ENH: do not exclude tests from distribution #34

Merged
merged 1 commit into from
Jun 28, 2015

Conversation

yarikoptic
Copy link
Contributor

Those are handy things to be distributed so users (or packagers such as yours truly) could verify correct operation of the library on their system... if accepted here I would appreciate if similar change was applied to scrapy as well

@kmike
Copy link
Member

kmike commented Jun 24, 2015

It will install 'tests' package to user packages, won't it?

@yarikoptic
Copy link
Contributor Author

d'oh -- you are totally right @kmike -- I rushed ;) what about starting to use yet another obscure feature of python distribution tools? (pushed another solution involving MANIFEST.in)

@yarikoptic
Copy link
Contributor Author

in general, I prefer (and it seems to be generally more common) when tests come as a part of the Python module itself (e.g. here under w3lib/tests). That allows people to test application "as installed". I just didn't want to impose anything. But now that I have introduced MANIFEST.in ... ;-)

@Digenis
Copy link
Member

Digenis commented Jun 25, 2015

in general, I prefer (and it seems to be generally more common) when tests come as a part of the Python module itself

Looks like the opposite happens here:
ac08e09
scrapy/scrapy@242c085
btw pytest's documentation accepts both schemes as correct

@dangra
Copy link
Member

dangra commented Jun 25, 2015

We tried the approach of shipping tests suite with the code, specially for Scrapy, but eventually problems arises like needing different dependencies to run tests than to run main app (i.e. mock, mitmproxy, py.test, ...).

I don't see distribution shipping tests suites of packages like apache, squid, bash, and so on, why scrapy related packages should be an exception?. I am aware distribution run test suites while building its packages, that's ok and can be done right now but packaging test suites so they are available in final systems is a different and unlikely to happen topic.

To be honest the idea is noble but not practical.

@yarikoptic
Copy link
Contributor Author

I hear you @dangra ! My reply is just for completeness -- I would respect any decision you take... hopefully at least you would ship tests/ within source distribution.

Additional dependencies are indeed a hassle, and different projects deal with it in various ways, most common approach just skip specific tests (or entire test module) if necessary dependency is absent (e.g. in PyMVPA we can run tests reliably without any external dependency but numpy although we have probably around 10 optional dependencies). Some (e.g. psychopy) also include complete py.test runner along to simplify running on some exotic systems without py.test being readily available.

As for comparison to other projects -- don't forget that we are quite special, since we are using Python, which come with builtin unittests module. Relatively small feature but imho it is the one fostered culture of better testing (on average ;-) ) among its projects. So, to my experience, majority of Python projects (numpy, scipy, pandas, sklearn, PyMVPA, even fail2ban now ;-), ...long list) ship with tests either under module/tests or even spread out per each submodule. Many of them even provide "module.test()" call-in to trigger testing.
Complementary to this, relying on tests being provided, such initiatives as DEP8 in the Debian world (http://dep.debian.net/deps/dep8) now blossoms with http://ci.debian.net/ which allows to identify causes for breakage while considering entire OS integration.
Hopefully this convinces you that it is actually quite practical. I agree though that it does require a tiny bit more of work.

@dangra
Copy link
Member

dangra commented Jun 26, 2015

I see your point, and aside of increase in tarball size I see no objections to ship tests/ dir in source .tar.gz distribution published to pypi. Right now for Scrapy we include docs/ and extras/ directories so nothing prevents us from including tests/ too. As long as binary distribution (wheel pkg) doesn't pack tests directory I am OK.

@dangra
Copy link
Member

dangra commented Jun 26, 2015

LGTM. @kmike feel free to merge.

@yarikoptic
Copy link
Contributor Author

Cool! Thanks! Then for the next release I will enable package build time testing for Debian package.
Should I submit similar one against scrapy or ... ? ;)

@dangra
Copy link
Member

dangra commented Jun 26, 2015

Cool! Thanks! Then for the next release I will enable package build time testing for Debian package.

That's great. thanks!

Should I submit similar one against scrapy or ... ? ;)

👍 go for it.

kmike added a commit that referenced this pull request Jun 28, 2015
ENH: do not exclude tests from distribution
@kmike kmike merged commit 9a7a3a7 into scrapy:master Jun 28, 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

4 participants