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 test_cli and test_cookies #2479

Merged
merged 9 commits into from Jun 19, 2022
Merged

Fix test_cli and test_cookies #2479

merged 9 commits into from Jun 19, 2022

Conversation

ChihweiLHBird
Copy link
Member

I was able to reproduce the test cli issues on my laptop, which is the timeout occurred when running the subprocess in capture function. After increasing the timeout locally, the issue never came up again during multiple tests.

Open this PR to further test the test cases.

I think this issue usually happen on smaller machine with lower performance.

@ChihweiLHBird ChihweiLHBird requested a review from a team as a code owner Jun 10, 2022
@codecov
Copy link

codecov bot commented Jun 10, 2022

Codecov Report

Merging #2479 (6da4c06) into main (ce926a3) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              main     #2479   +/-   ##
=========================================
  Coverage   87.267%   87.267%           
=========================================
  Files           60        60           
  Lines         5097      5097           
  Branches       913       913           
=========================================
  Hits          4448      4448           
  Misses         476       476           
  Partials       173       173           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce926a3...6da4c06. Read the comment docs.

@ChihweiLHBird ChihweiLHBird changed the title Increase Subprocess Timeout for Fixing Cli Testing Increase Subprocess Timeout to Fix test_cli Jun 11, 2022
@ChihweiLHBird
Copy link
Member Author

Test cookies issue seems a problem of httpx's async client.
encode/httpx#2144

In the failed test cookie case, the headers contain setting cookie but hpptx didn't put it into response.cookies. I think there is not much we can do until httpx dev team fix it. If we really want to fix it right now, we can try to parse the cookies in sanic-testing after getting the response object from httpx.

ahopkins
ahopkins previously approved these changes Jun 17, 2022
@ChihweiLHBird
Copy link
Member Author

ChihweiLHBird commented Jun 17, 2022

It's weird that fixing cli tests appears increase the chance of the failed cookies test. @ahopkins

@ChihweiLHBird
Copy link
Member Author

The cookies test fixed, the only (occasionally) broken test case is keep alive.

@ChihweiLHBird ChihweiLHBird requested a review from ahopkins Jun 19, 2022
@ChihweiLHBird ChihweiLHBird changed the title Increase Subprocess Timeout to Fix test_cli Fix test_cli and test_cookies Jun 19, 2022
@ahopkins ahopkins merged commit d1c5e80 into main Jun 19, 2022
27 checks passed
@ahopkins ahopkins deleted the ChihweiLHBird-patch-1 branch Jun 19, 2022
@ahopkins
Copy link
Member

Amazing amazing work. You rock.

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.

None yet

2 participants