Skip to content

Conversation

mabdinur
Copy link
Contributor

@mabdinur mabdinur commented Nov 1, 2022

Description

The following tests asserts an asyncpg connection is garbage collected. This test is incompatible with the profiler and should be skipped: test_no_explicit_close_with_debug

Checklist

Motivation

Design

Testing strategy

Relevant issue(s)

Testing strategy

Reviewer Checklist

  • Title is accurate.
  • Description motivates each change.
  • No unnecessary changes were introduced in this PR.
  • Avoid breaking API changes unless absolutely necessary.
  • Tests provided or description of manual testing performed is included in the code or PR.
  • Release note has been added for fixes and features, or else changelog/no-changelog label added.
  • All relevant GitHub issues are correctly linked.
  • Backports are identified and tagged with Mergifyio.

@mabdinur mabdinur marked this pull request as ready for review November 1, 2022 21:21
@mabdinur mabdinur requested a review from a team as a code owner November 1, 2022 21:21
@mabdinur mabdinur added the changelog/no-changelog A changelog entry is not required for this PR. label Nov 1, 2022
Copy link
Member

@Kyle-Verhoog Kyle-Verhoog left a comment

Choose a reason for hiding this comment

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

what change caused this? is this a new test in the asyncpg suite or a change we made to the profiler?

@mabdinur
Copy link
Contributor Author

mabdinur commented Nov 2, 2022

what change caused this? is this a new test in the asyncpg suite or a change we made to the profiler?

@jd do you know of any recent changes that would have caused this test to fail?

@Kyle-Verhoog
Copy link
Member

@mabdinur how do we know it's profiling related?

@Kyle-Verhoog
Copy link
Member

image
looks like there was a change in asyncpg but I haven't looked yet to see if it's flaky on their side too or if ddtrace is responsible.

@Kyle-Verhoog
Copy link
Member

seems like it could be #4125 based on @Yun-Kim and I's findings. It lines up with the CI starting to flake too.

@Yun-Kim
Copy link
Contributor

Yun-Kim commented Nov 2, 2022

Failing framework test fixed by #4448.

@Yun-Kim Yun-Kim closed this Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/no-changelog A changelog entry is not required for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants