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

Pass info from FEEDS to ItemExporter #4768

Merged
merged 4 commits into from Oct 1, 2020

Conversation

maranqz
Copy link
Contributor

@maranqz maranqz commented Aug 30, 2020

Resolves #4606

maranqz added 3 commits Aug 30, 2020
 * FeedExportConfigTestCase.test_feed_complete_default_values_from_settings_empty
 * FeedExportConfigTestCase.test_feed_complete_default_values_from_settings_non_empty
@codecov
Copy link

codecov bot commented Aug 30, 2020

Codecov Report

Merging #4768 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4768      +/-   ##
==========================================
+ Coverage   87.19%   87.21%   +0.02%     
==========================================
  Files         160      160              
  Lines        9813     9814       +1     
  Branches     1447     1447              
==========================================
+ Hits         8556     8559       +3     
+ Misses        995      994       -1     
+ Partials      262      261       -1     
Impacted Files Coverage Δ
scrapy/extensions/feedexport.py 95.01% <ø> (ø)
scrapy/utils/conf.py 93.75% <100.00%> (+0.05%) ⬆️
scrapy/core/downloader/__init__.py 92.48% <0.00%> (+1.50%) ⬆️

@maranqz
Copy link
Contributor Author

maranqz commented Aug 30, 2020

The second commit was fail. In third one I do nothing and tests passed. I think it is not correct.

Copy link
Member

@Gallaecio Gallaecio left a comment

In third one I do nothing and tests passed. I think it is not correct.

I don’t know what you mean. If I comment out the feedexport.py line, the new test fails as expected.

As far as I can tell, good work!

docs/topics/feed-exports.rst Outdated Show resolved Hide resolved
Co-authored-by: Adrián Chaves <adrian@chaves.io>
@maranqz
Copy link
Contributor Author

maranqz commented Sep 1, 2020

In third one I do nothing and tests passed. I think it is not correct.

I don’t know what you mean. If I comment out the feedexport.py line, the new test fails as expected.

As far as I can tell, good work!

I mean, there is problem with tests.
Testing of commit fc3c66c was fail. Then I create 71d2c2f without valuable changes and tests passed.

@Gallaecio
Copy link
Member

Gallaecio commented Sep 1, 2020

I mean, there is problem with tests.
Testing of commit fc3c66c was fail. Then I create 71d2c2f without valuable changes and tests passed.

Oh, I see! I thought you meant there was a problem in your code, not in the CI. My bad!

Looks like it was a case of random failure. We have a few of those related to the logging of warnings and errors, sometimes they pick unrelated logging messages causing a test to fail. Nothing for you to worry about.

@kmike
Copy link
Member

kmike commented Oct 1, 2020

Thanks @maranqz!

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.

Pass info from FEEDS to ItemExporter
3 participants