Skip to content

Conversation

@hugovk
Copy link
Member

@hugovkhugovk commented Sep 8, 2023

Fixes#106193.
Found via python/core-workflow#505 (comment).

Fixes some copy/paste errors, and then fix some undefined names, because the redefined method name masked the unknown variables. We can work out what they should be.

Before

We can see two pairs of tests:

@staticmethoddeffunc1(): line1=1MUST_INCLUDE_LI= [ ('instruction', 'func1', 2), ('line', 'func1', 1), ('instruction', 'func1', 4), ('instruction', 'func1', 6)] deftest_line_then_instruction(self): recorders= [ LineRecorder, InstructionRecorder ] self.check_events(self.func1, recorders=recorders, must_include=self.EXPECTED_LI) deftest_instruction_then_line(self): recorders= [ InstructionRecorder, LineRecorderLowNoise ] self.check_events(self.func1, recorders=recorders, must_include=self.EXPECTED_LI)
@staticmethoddeffunc2(): len(()) MUST_INCLUDE_CI= [ ('instruction', 'func2', 2), ('call', 'func2', sys.monitoring.MISSING), ('call', 'len', ()), ('instruction', 'func2', 12), ('instruction', 'func2', 14)] deftest_line_then_instruction(self): recorders= [ CallRecorder, InstructionRecorder ] self.check_events(self.func2, recorders=recorders, must_include=self.MUST_INCLUDE_CI) deftest_instruction_then_line(self): recorders= [ InstructionRecorder, CallRecorder ] self.check_events(self.func2, recorders=recorders, must_include=self.MUST_INCLUDE_CI)

After

Fixes:

  • Test names are based on the recorders.
  • self.EXPECTED_LI should be self.MUST_INCLUDE_CI, like the others.
  • LineRecorderLowNoise should be LineRecorder, like the previous test.
  • ('line', 'func1', 1) needed changing to ('line', 'func1', 2) to pass.
@staticmethoddeffunc1(): line1=1MUST_INCLUDE_LI= [ ('instruction', 'func1', 2), ('line', 'func1', 2), ('instruction', 'func1', 4), ('instruction', 'func1', 6)] deftest_line_then_instruction(self): recorders= [ LineRecorder, InstructionRecorder ] self.check_events(self.func1, recorders=recorders, must_include=self.MUST_INCLUDE_LI) deftest_instruction_then_line(self): recorders= [ InstructionRecorder, LineRecorder ] self.check_events(self.func1, recorders=recorders, must_include=self.MUST_INCLUDE_LI)
@staticmethoddeffunc2(): len(()) MUST_INCLUDE_CI= [ ('instruction', 'func2', 2), ('call', 'func2', sys.monitoring.MISSING), ('call', 'len', ()), ('instruction', 'func2', 12), ('instruction', 'func2', 14)] deftest_call_then_instruction(self): recorders= [ CallRecorder, InstructionRecorder ] self.check_events(self.func2, recorders=recorders, must_include=self.MUST_INCLUDE_CI) deftest_instruction_then_call(self): recorders= [ InstructionRecorder, CallRecorder ] self.check_events(self.func2, recorders=recorders, must_include=self.MUST_INCLUDE_CI)

cc @markshannon@sobolevn

@hugovk
Copy link
MemberAuthor

(Resolved conflict.)

@markshannon
Copy link
Member

Thanks

@markshannonmarkshannon merged commit ea530f2 into python:mainOct 12, 2023
@miss-islington
Copy link
Contributor

Thanks @hugovk for the PR, and @markshannon for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islington
Copy link
Contributor

Sorry, @hugovk and @markshannon, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker ea530f2f9ae63e81c22a1818bec0a650ccf758d2 3.12 

@hugovkhugovk deleted the fix-duplicate-names-test_monitoring branch October 12, 2023 22:24
@bedevere-app
Copy link

GH-110897 is a backport of this pull request to the 3.12 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.12 only security fixes label Oct 15, 2023
hugovk added a commit to hugovk/cpython that referenced this pull request Oct 15, 2023
…toring` (pythonGH-109139) (cherry picked from commit ea530f2) Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip newstestsTests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_monitoring has duplicated tests

4 participants

@hugovk@markshannon@miss-islington@bedevere-bot