scrapy / scrapy

Scrapy, a fast high-level web crawling & scraping framework for Python.
https://scrapy.org
BSD 3-Clause "New" or "Revised" License
50.98k stars 10.34k forks source link

chore: Removing tests/requirements.txt and adding dependencies to the tox.ini file #6272

Closed lucas-belo closed 1 month ago

lucas-belo commented 2 months ago

Resolves https://github.com/scrapy/scrapy/issues/6270

Gallaecio commented 2 months ago

Note that you’ll need to resolve conflicts.

lucas-belo commented 2 months ago

Trying to find the lowest version of the packages with which tests pass, with a binary search strategy among all the packages versions.

codecov[bot] commented 1 month ago

Codecov Report

Merging #6272 (e91d63e) into master (95a70d3) will increase coverage by 0.10%. The diff coverage is n/a.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #6272 +/- ## ========================================== + Coverage 88.63% 88.73% +0.10% ========================================== Files 161 161 Lines 11971 11971 Branches 1929 1929 ========================================== + Hits 10610 10622 +12 + Misses 1005 993 -12 Partials 356 356 ``` [see 2 files with indirect coverage changes](https://app.codecov.io/gh/scrapy/scrapy/pull/6272/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy)
Gallaecio commented 1 month ago

Waiting for CI, given we have to manually approve every time, might be too time-consuming. You can run tests locally with tox, e.g. tox -e py for regular tests with your system version of Python, tox -e extra-deps-pinned and tox -e extra-deps for the extra-deps environments, etc.

lucas-belo commented 1 month ago

Hey @Gallaecio and @wRAR, all tests were successful for the last commit on my fork's CI, is there any difference? Maybe re-run the failed tests?

https://github.com/lucas-belo/scrapy/actions/runs/8211648105 image

wRAR commented 1 month ago

3.8 tests fail because of #6286

lucas-belo commented 1 month ago

Great work!

I think we can merge after a run of pre-commit run --all to fix the pre-commit CI job, and provided no other unexpected CI job failures.

@Gallaecio, Done!

https://github.com/lucas-belo/scrapy/actions/runs/8249523479 image

Gallaecio commented 1 month ago

I think the issues in extra-deps-pinned may be real issues. We might need a higher version of some extra dep.

wRAR commented 1 month ago

Of uvloop, it seems: https://github.com/MagicStack/uvloop/issues/126

lucas-belo commented 1 month ago

I am gonna see it

lucas-belo commented 1 month ago

With uvloop==0.12.0, all tests passed, but we got 4 erros:

image

lucas-belo commented 1 month ago

With uvloop==0.14.0 we got all tests successful and no errors, I think this is the right one:

image

Gallaecio commented 1 month ago

I have added pygments to deps, and I squashed your commits because, although git diff scrapy/master showed the right changes, the GitHub diff included 30 files*, making review online a bit harder.

* As to why it happened, I believe either because you merged a PR of mine to fix CI jobs, which I then squash-merged into the main branch, or due to the way you merged/rebased the main branch; doesn’t not really matter.

Gallaecio commented 1 month ago

@lucas-belo Thank you for your amazing work here!