Disable retry in tests

[Dreamsorcerer]

The client now has retry logic (

aiohttp/aiohttp/client.py

Line 642 in ba51537

if retry_persistent_connection:

), but this logic can mask issues in tests.
We should disable the retry logic by default in tests (e.g. when using TestClient).

[ShubhAgarwal-dev]

Can I be assigned this? I will try to get this as part of the hacktoberfest event...

[Dreamsorcerer]

Go ahead.

[ShubhAgarwal-dev]

Hi Sam, this is my first time contributing to open-source and working with such a huge codebase. Can you guide me a bit on what is required to be done..?

[webknjaz]

@ShubhAgarwal-dev this needs understanding how the tests work, inspecting them and making sure they disable retries, essentially not hitting the block under the linked line of code. Perhaps, running the tests locally and generating an HTML coverage report would reveal which tests hit said line. What's needed is probably finding where TestClient is defined in code and making sure it defaults to initializing the client session with retries disabled.

@Dreamsorcerer perhaps we shouldn't mark issues as Hacktoberfest when they require deeper context?

[Dreamsorcerer]

Well, basically we want the linked line in the original post to always be False. So, we could probably add a new argument to ClientSession, or a private attribute maybe which defaults that variable to True normally, but False in the tests.

The ClientSession for TestClient is created at:

aiohttp/aiohttp/test_utils.py

Line 296 in 25b67b6

self._session = ClientSession(cookie_jar=cookie_jar, **kwargs)

So, that's where we'll need to pass the parameter or modify the private attribute.

@Dreamsorcerer perhaps we shouldn't mark issues as Hacktoberfest when they require deeper context?

I think the above is sufficient, no need to verify coverage etc. or have deeper knowledge of the code. Once a rough implementation is ready, we can figure out a test to verify the behaviour.

[Dreamsorcerer]

I'd probably suggest the private attribute initially, and keep it as an implementation detail. If users ask for this to be configurable in future, then we can consider adding a new parameter.

[ShubhAgarwal-dev]

Is there anything left in this issue..?

[Dreamsorcerer]

Ah, forgot to update the PR. If you include 'Fixes #9141' in the PR description next time, then the issue would be auto-closed.