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

Missing cssselect and mock requirements. #446

Closed
wants to merge 3 commits into from

Conversation

rmax
Copy link
Contributor

@rmax rmax commented Oct 28, 2013

The cssselect module was missing from the requirements.txt. The mock module is needed to run one test in the tests suite, that module was already added to the travis' requirements but is missing from any requirements file in the root directory.

As is common in another projects, the requirements-tests.txt was created to specifically hold the modules required for running the tests. This does not include optional modules.

Also, .travis/requirements-lucid.txt was not modified as python 2.6 support is going to be dropped and it's most likely cssselect does not work with lxml 2.2.x.

requirements-lucid was not modified as perhaps cssselect is not going to
work with lxml 2.2.x and python 2.6 support is not longer supported
anyway.
As test_pipeline_files requires mock module, the file requirements-
tests.txt was created to hold modules *needed* to run the tests.

Travis' requirements already includes the mock module, but there is no
hint in the root tree that mock module is required. This new file will
help new contributors to quickly find which modules are required to run
the tests.
@rmax
Copy link
Contributor Author

rmax commented Oct 28, 2013

Oh, it seems travis installs cssselect when running the lucid environment even though the requirements-lucid does not include cssselect: https://travis-ci.org/scrapy/scrapy/jobs/13131752#L340

Is that right?

Edit: That's because the requirements are in the setup.py too. I see that travis' requirements serve the purpose to restrict to an specific version. In that case, isn't redundant having the same requirements in .travis/requirements-latest.txt as in setup.py? (not all of them, just the ones that are in both.)

@dangra
Copy link
Member

dangra commented Oct 29, 2013

.travis/requirements-ENV.txt and setup.py serves different purposes, setup.py must list all dependencies and its minimum versions to install from pypi.

While travis is used to test Scrapy with specific python package versions installed as system packages in distributions (lucid and precise). But also to test most recent stable versions (latest) or development versions (trunk) of requirements.

cssselect is required but it doesn't exists as system package in ubuntu lucid or precise, Scrapy apt repositories serves the version 0.9.1 at the moment (not for lucid whose support is going to be removed in Scrapy 0.20)

that said, you are right about requirements-latest.txt duplicating setup.py requirements, do you want to make the change?

@rmax
Copy link
Contributor Author

rmax commented Oct 29, 2013

@dangra please take the lead, I'm bit busy this days.

@dangra
Copy link
Member

dangra commented Nov 19, 2013

fixed as part of 6051612

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 1e1caa7 on darkrho:requirements into * on scrapy:master*.

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

3 participants