Skip to content

Fix for #5855 #5898

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
merged 8 commits into from
Apr 19, 2023
Merged

Fix for #5855 #5898

merged 8 commits into from
Apr 19, 2023

Conversation

tstauder
Copy link
Contributor

@tstauder tstauder commented Apr 17, 2023

Changed batch_time to batch_id in test_batch_path_differ function in test_feedexport: #5855

Fixes #5855

@Gallaecio
Copy link
Member

#5855 (comment)

@tstauder First suggestion: review your changes as seen in your pull request, and make sure they only include intended changes. I see some commented out code that you probably used during debugging and should not be a part of this change. I also see a bad edit of a docstring.

@codecov
Copy link

codecov bot commented Apr 17, 2023

Codecov Report

Merging #5898 (a5cec93) into master (e1f6662) will decrease coverage by 0.53%.
The diff coverage is 93.05%.

❗ Current head a5cec93 differs from pull request most recent head b30670c. Consider uploading reports for the commit b30670c to get more accurate results

@@            Coverage Diff             @@
##           master    #5898      +/-   ##
==========================================
- Coverage   88.82%   88.30%   -0.53%     
==========================================
  Files         162      162              
  Lines       11129    11052      -77     
  Branches     1808     1637     -171     
==========================================
- Hits         9885     9759     -126     
- Misses        961      997      +36     
- Partials      283      296      +13     
Impacted Files Coverage Δ
scrapy/utils/python.py 88.67% <81.81%> (-1.00%) ⬇️
scrapy/core/downloader/handlers/__init__.py 81.35% <87.50%> (-11.38%) ⬇️
scrapy/core/downloader/tls.py 94.44% <87.50%> (-5.56%) ⬇️
scrapy/core/downloader/contextfactory.py 85.00% <90.00%> (-2.50%) ⬇️
scrapy/core/downloader/webclient.py 94.77% <95.23%> (+0.11%) ⬆️
scrapy/core/downloader/__init__.py 91.72% <95.65%> (-1.09%) ⬇️
scrapy/core/downloader/middleware.py 96.77% <100.00%> (+0.05%) ⬆️
scrapy/utils/ssl.py 56.81% <100.00%> (-0.69%) ⬇️

... and 65 files with indirect coverage changes

@tstauder
Copy link
Contributor Author

tstauder commented Apr 17, 2023

Thanks for the helpful feedback. I cleaned up those issues and updated the pull request to include them.

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.

Other than the minor issue I mention below, it looks good to me. Thanks!

@Gallaecio Gallaecio merged commit ef61fb5 into scrapy:master Apr 19, 2023
@Gallaecio
Copy link
Member

Thanks!

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.

test_batch_path_differ sometimes fails
4 participants