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

fix: docs/intro/tutorial.rst checks #5902

Merged
merged 7 commits into from
Apr 21, 2023
Merged

Conversation

jxlil
Copy link
Contributor

@jxlil jxlil commented Apr 19, 2023

Fix docs/intro/tutorial.rst checks:

command:

pytest docs

output:

FAILED docs/intro/tutorial.rst::line:254,column:1
FAILED docs/intro/tutorial.rst::line:350,column:1
FAILED docs/intro/tutorial.rst::line:412,column:1

@codecov
Copy link

codecov bot commented Apr 19, 2023

Codecov Report

Merging #5902 (718dbeb) into master (02f3e8d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 718dbeb differs from pull request most recent head 992b950. Consider uploading reports for the commit 992b950 to get more accurate results

@@           Coverage Diff           @@
##           master    #5902   +/-   ##
=======================================
  Coverage   88.77%   88.77%           
=======================================
  Files         162      162           
  Lines       11159    11165    +6     
  Branches     1813     1814    +1     
=======================================
+ Hits         9906     9912    +6     
  Misses        966      966           
  Partials      287      287           
Impacted Files Coverage Δ
scrapy/utils/curl.py 100.00% <100.00%> (ø)

@Gallaecio
Copy link
Member

Gallaecio commented Apr 19, 2023

I wonder what to do with pinned environments.

Maybe we could skip doctests with pinned versions:

  1. Remove --doctest-modules from pytest.ini
  2. Add it to tox.ini’s default command
  3. Add custom commands to the 4 pinned environments in tox.ini without --doctest-modules

@Gallaecio Gallaecio self-requested a review April 19, 2023 06:50
@jxlil jxlil marked this pull request as ready for review April 19, 2023 22:06
@jxlil
Copy link
Contributor Author

jxlil commented Apr 19, 2023

@Gallaecio I made the changes you mention, it seems that there are no more failures related to tutorial.rst

tox.ini Outdated
@@ -152,7 +156,7 @@ basepython = {[testenv:pypy3]basepython}
deps =
{[pinned]deps}
PyPyDispatcher==2.1.0
commands = {[testenv:pypy3]commands}
commands = {[pinned]commands}
Copy link
Member

Choose a reason for hiding this comment

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

This was probably OK before, testenv:pypy3 above does not use --doctest-modules. I suspect this change may be responsible for the job failing now (maybe coverage breaks pypy?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, how strange it seems that now all the tests are passing, but I remember that one was failing.

Anyway, you're right. I'm going to undo this change. I see that previously the command did not use --doctest-modules nor did it use coverage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I undid the change, but the test failed again. So I added a custom command for pypy3-pinned

Copy link
Member

Choose a reason for hiding this comment

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

I remember that one was failing

I did re-run the failing test in case it was a random issue, but I did not expect it to pass 😅

@Gallaecio
Copy link
Member

Nice, thanks!

@Gallaecio Gallaecio merged commit 5a37af1 into scrapy:master Apr 21, 2023
@jxlil jxlil deleted the fix/tutorial.rst branch April 21, 2023 16:38
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.

2 participants