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

docs: Fix statement about setUpAsync() and tearDownAsync() #6313

Merged
merged 1 commit into from
Nov 16, 2021

Conversation

H--o-l
Copy link
Contributor

@H--o-l H--o-l commented Nov 15, 2021

Since #4732, it's wrong to says that
setUpAsync() and tearDownAsync() do nothing and can be overridden without
calling super.setUpAsync() and super.tearDownAsync().

What do these changes do?

Fix documentation

Are there changes in behavior for the user?

No

Related issue number

None

Checklist

Only docs.


Remark:
My rst preview wasn't working, I hope there is no rst mistakes.

docs/testing.rst Outdated Show resolved Hide resolved
docs/testing.rst Outdated Show resolved Hide resolved
@Dreamsorcerer
Copy link
Member

Looks like a backwards-incompatible change we made by mistake.
We should probably point this out in the change fragment.

@codecov
Copy link

codecov bot commented Nov 15, 2021

Codecov Report

Merging #6313 (863182e) into master (14f1854) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6313   +/-   ##
=======================================
  Coverage   93.31%   93.31%           
=======================================
  Files         103      103           
  Lines       30369    30369           
  Branches     2731     2731           
=======================================
  Hits        28340    28340           
  Misses       1852     1852           
  Partials      177      177           
Flag Coverage Δ
unit 93.24% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


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 14f1854...863182e. Read the comment docs.

@H--o-l
Copy link
Contributor Author

H--o-l commented Nov 16, 2021

Looks like a backwards-incompatible change we made by mistake.

I agree.

We should probably point this out in the change fragment.

It's something I should do in this PR?

Thanks @Dreamsorcerer for your first review!

@H--o-l
Copy link
Contributor Author

H--o-l commented Nov 16, 2021

Open question, maybe

           async def tearDownAsync(self):
               await foo()
               await super().tearDownAsync()

Would be more suited than

           async def tearDownAsync(self):
               await super().tearDownAsync()
               await foo()

?

docs/testing.rst Show resolved Hide resolved
docs/testing.rst Show resolved Hide resolved
Since aio-libs#4732, it's wrong to says that
`setUpAsync()` and `tearDownAsync()` do nothing and can be overridden without
calling `super.setUpAsync()` and `super.tearDownAsync()`.
Copy link
Member

@asvetlov asvetlov left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@asvetlov asvetlov added backport-3.8 backport-3.9 Trigger automatic backporting to the 3.9 release branch by Patchback robot bot:chronographer:skip This PR does not need to include a change note labels Nov 16, 2021
@asvetlov asvetlov enabled auto-merge (squash) November 16, 2021 14:00
@asvetlov asvetlov merged commit d149eff into aio-libs:master Nov 16, 2021
@patchback
Copy link
Contributor

patchback bot commented Nov 16, 2021

Backport to 3.8: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.8/d149eff62e1e110d2eafac8a79219dd2e56b8581/pr-6313

Backported as #6317

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Nov 16, 2021
Since #4732, it's wrong to says that
`setUpAsync()` and `tearDownAsync()` do nothing and can be overridden without
calling `super.setUpAsync()` and `super.tearDownAsync()`.

(cherry picked from commit d149eff)
@patchback
Copy link
Contributor

patchback bot commented Nov 16, 2021

Backport to 3.9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.9/d149eff62e1e110d2eafac8a79219dd2e56b8581/pr-6313

Backported as #6318

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Nov 16, 2021
Since #4732, it's wrong to says that
`setUpAsync()` and `tearDownAsync()` do nothing and can be overridden without
calling `super.setUpAsync()` and `super.tearDownAsync()`.

(cherry picked from commit d149eff)
asvetlov pushed a commit that referenced this pull request Nov 16, 2021
#6318)

Since #4732, it's wrong to says that
`setUpAsync()` and `tearDownAsync()` do nothing and can be overridden without
calling `super.setUpAsync()` and `super.tearDownAsync()`.

(cherry picked from commit d149eff)

Co-authored-by: Hoel IRIS <hoel.iris@gmail.com>
asvetlov pushed a commit that referenced this pull request Nov 16, 2021
#6317)

Since #4732, it's wrong to says that
`setUpAsync()` and `tearDownAsync()` do nothing and can be overridden without
calling `super.setUpAsync()` and `super.tearDownAsync()`.

(cherry picked from commit d149eff)

Co-authored-by: Hoel IRIS <hoel.iris@gmail.com>
@H--o-l H--o-l deleted the docs/setUpAsync branch November 16, 2021 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-3.9 Trigger automatic backporting to the 3.9 release branch by Patchback robot bot:chronographer:skip This PR does not need to include a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants