Skip to content

Issue #3264, fix error handling when spider is not matched #5497

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

Merged

Conversation

andreastziortz
Copy link
Contributor

@andreastziortz andreastziortz commented May 6, 2022

Changes
Implementation:

  • Check whether Spider exists or is None, and if it's None skip execution of start_requests() with non existing Spider

Testing:

  • Add a test case with invalid url inside test_command_parse
    Test proves that non-matched Spider does not throw an AttributeError

Fixes #3264, resolves #5376

Changes
Implementation:
- Check whether Spider exists or is None, and if it's None skip execution of start_requests() with non existing Spider
Testing:
- Add a test case with invalid url inside test_command_parse
  Test proves that non-matched Spider does not throw an AttributeError
@@ -222,6 +223,10 @@ def test_crawlspider_no_matching_rule(self):
self.assertRegex(_textmode(out), r"""# Scraped Items -+\n\[\]""")
self.assertIn("""Cannot find a rule that matches""", _textmode(stderr))

status, out, stderr = yield self.execute([self.url('/invalid_url')])
Copy link
Member

Choose a reason for hiding this comment

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

Please, give this test its own test method instead of running it within the existing test_crawlspider_no_matching_rule.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @Gallaecio , should I use also the decorator @defer.inlineCallbacks in my new test?

@codecov
Copy link

codecov bot commented May 24, 2022

Codecov Report

Merging #5497 (b5c15d8) into master (ded28f7) will increase coverage by 0.15%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #5497      +/-   ##
==========================================
+ Coverage   88.58%   88.74%   +0.15%     
==========================================
  Files         163      163              
  Lines       10666    10671       +5     
  Branches     1818     1819       +1     
==========================================
+ Hits         9449     9470      +21     
+ Misses        941      926      -15     
+ Partials      276      275       -1     
Impacted Files Coverage Δ
scrapy/commands/parse.py 26.81% <0.00%> (-0.16%) ⬇️
scrapy/pipelines/files.py 71.23% <0.00%> (-0.65%) ⬇️
scrapy/extensions/statsmailer.py 0.00% <0.00%> (ø)
scrapy/core/downloader/__init__.py 92.48% <0.00%> (+1.50%) ⬆️
scrapy/robotstxt.py 97.53% <0.00%> (+22.22%) ⬆️

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.

Looks good to me, thanks!

@kmike kmike merged commit 99cddec into scrapy:master May 28, 2022
@kmike
Copy link
Member

kmike commented May 28, 2022

Thanks @andreastziortz!

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.

Command parse unhandled error :AttributeError: 'NoneType' object has no attribute 'start_requests'
3 participants