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

[MRG+1] Add SCRAPY_CHECK environment variable #3739

Merged
merged 10 commits into from
Jun 13, 2019

Conversation

Matthijsy
Copy link
Contributor

@Matthijsy Matthijsy commented Apr 10, 2019

This PR implements the environment variable when running scrapy check as asked in #3704 . This way it is possible to have different behaviour in the scraper when running the check (like requiring less settings to be set)

fixes #3704

@codecov
Copy link

codecov bot commented Apr 10, 2019

Codecov Report

Merging #3739 into master will increase coverage by <.01%.
The diff coverage is 54.54%.

@@            Coverage Diff             @@
##           master    #3739      +/-   ##
==========================================
+ Coverage   85.42%   85.43%   +<.01%     
==========================================
  Files         169      169              
  Lines        9635     9647      +12     
  Branches     1433     1436       +3     
==========================================
+ Hits         8231     8242      +11     
- Misses       1156     1157       +1     
  Partials      248      248
Impacted Files Coverage Δ
scrapy/utils/misc.py 96.29% <100%> (+0.58%) ⬆️
scrapy/commands/check.py 25.35% <9.09%> (-0.37%) ⬇️

@Gallaecio
Copy link
Member

Could you please add an automated test to cover this change?

@Matthijsy Matthijsy closed this Apr 18, 2019
@Matthijsy Matthijsy reopened this Apr 18, 2019
@Matthijsy
Copy link
Contributor Author

I've added a test for the set_environ util. I couldn't find the test of scrapy check so don't know how I should write a test for that part. Is this untested or can you tell me where I can find it?

@Gallaecio
Copy link
Member

scrapy/tests/test_cmdline/__init__.py tests some commands, but there are no tests for scrapy check currently, no, so I’m OK with your test coverage.

It would be great if you could cover the new environment variable in the documentation about scrapy check. Other than that, I think this is ready to merge.

@Matthijsy
Copy link
Contributor Author

@Gallaecio Thanks for your feedback! I've added the documentation, hope this is the right way to document it and otherwise I would love to update the documentation.

Copy link
Member

@Gallaecio Gallaecio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, build the documentation locally to see how your changes look.

To build the documentation, you can create a virtual environment with the dependencies from requirements-py3.txt and docs/requirements.txt, and run make html from the docs folder. You can then open docs/build/html/index.html in a web browser and reach the contracts page.

scrapy/utils/misc.py Show resolved Hide resolved
scrapy/utils/misc.py Outdated Show resolved Hide resolved
tests/test_utils_misc/__init__.py Outdated Show resolved Hide resolved
@Matthijsy
Copy link
Contributor Author

Thanks for the comment on how to build the documentation! I builded it and think it looks fine now

@Gallaecio
Copy link
Member

I’ve refactored your documentation a bit. Please, review my changes.

@Gallaecio Gallaecio changed the title Add SCRAPY_CHECK environment variable [MRG+1] Add SCRAPY_CHECK environment variable May 3, 2019
@Matthijsy
Copy link
Contributor Author

Great! Looks fine to me

@dangra dangra merged commit cdfb20a into scrapy:master Jun 13, 2019
@dangra dangra added this to the v1.7 milestone Jun 13, 2019
@Matthijsy Matthijsy deleted the feature/scrapy_check_env branch October 7, 2019 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set environment variable when running scrapy check
3 participants