Skip to content

Remove explicit passing of event_loop into tests#1006

Merged
jujubot merged 3 commits intojuju:masterfrom
cderici:clean-up-warnings
Jan 11, 2024
Merged

Remove explicit passing of event_loop into tests#1006
jujubot merged 3 commits intojuju:masterfrom
cderici:clean-up-warnings

Conversation

@cderici
Copy link
Copy Markdown
Contributor

@cderici cderici commented Jan 9, 2024

Description

This clears up from the test output the flood of warnings from pytest that looks like (e.g. example):

tests/unit/test_bundle.py:738
  tests/unit/test_bundle.py:738: PytestDeprecationWarning: test_run is asynchronous and explicitly requests the "event_loop" fixture. Asynchronous fixtures and test functions should use "asyncio.get_running_loop()" instead.
    @pytest.mark.asyncio

QA Steps

No functionality changes. Though there were a couple of tests that I needed to manually get the running loop (where the test actually was using the event_loop), so we need to make sure those are still passing.

Notes & Discussions

Maybe need to be back-ported? I'm not sure yet.

@cderici cderici added the hint/3.x going on main branch label Jan 9, 2024
@cderici
Copy link
Copy Markdown
Contributor Author

cderici commented Jan 10, 2024

Failures in CI are non-related.

TypeError: HTTPConnection.request() got an unexpected keyword argument 'chunked' on is being fixed in #1005, the rest it known intermittent issues that will be addressed later.

@cderici cderici requested review from anvial and jack-w-shaw January 10, 2024 20:20
Copy link
Copy Markdown
Member

@jack-w-shaw jack-w-shaw left a comment

Choose a reason for hiding this comment

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

LGTM presuming integration tests pass

@jack-w-shaw
Copy link
Copy Markdown
Member

Should this be targeting 2.9, and be merged forward?

@cderici
Copy link
Copy Markdown
Contributor Author

cderici commented Jan 11, 2024

Should this be targeting 2.9, and be merged forward?

I didn't think that it was happening in 2.9, but looks like it is. Well we can backport this no problem I think because there's no functionality change, just removing an argument wouldn't result it any conflicts.

@cderici cderici added the area/backward-port to be backward ported label Jan 11, 2024
@cderici
Copy link
Copy Markdown
Contributor Author

cderici commented Jan 11, 2024

/merge

@jujubot jujubot merged commit 3bb530d into juju:master Jan 11, 2024
@cderici cderici mentioned this pull request Feb 8, 2024
jujubot added a commit that referenced this pull request Feb 13, 2024
#1024

## What's Changed
* Remove paramiko upper-bound by @gboutry in #1005
* Remove explicit passing of event_loop into tests by @cderici in #1006
* chore: remove the upper restrictions on the websockets dependency by @tonyandrewmeyer in #1007
* Target ceiling version by @cderici in #1008
* Make it easier to run tests using `make` by @cderici in #1012
* Avoid installing signal handlers to the event loop by @cderici in #1014
* feat: remove app block until done by @yanksyoon in #1017
* feat: remove app timeout by @yanksyoon in #1018
* Forward port latest changes from 2.9 onto 3.x by @cderici in #1022

#### Notes & Discussion

JUJU-5414
@cderici cderici mentioned this pull request Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/backward-port to be backward ported hint/3.x going on main branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants