Skip to content

gh-106193: Rename and fix duplicated tests in test_monitoring #109139

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Oct 12, 2023
Merged

gh-106193: Rename and fix duplicated tests in test_monitoring #109139

merged 4 commits into from
Oct 12, 2023

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Sep 8, 2023 •
edited
Loading

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:

    @staticmethod
    def func1():
        line1 = 1

    MUST_INCLUDE_LI = [
            ('instruction', 'func1', 2),
            ('line', 'func1', 1),
            ('instruction', 'func1', 4),
            ('instruction', 'func1', 6)]

    def test_line_then_instruction(self):
        recorders = [ LineRecorder, InstructionRecorder ]
        self.check_events(self.func1,
                          recorders = recorders, must_include = self.EXPECTED_LI)

    def test_instruction_then_line(self):
        recorders = [ InstructionRecorder, LineRecorderLowNoise ]
        self.check_events(self.func1,
                          recorders = recorders, must_include = self.EXPECTED_LI)
    @staticmethod
    def func2():
        len(())

    MUST_INCLUDE_CI = [
            ('instruction', 'func2', 2),
            ('call', 'func2', sys.monitoring.MISSING),
            ('call', 'len', ()),
            ('instruction', 'func2', 12),
            ('instruction', 'func2', 14)]



    def test_line_then_instruction(self):
        recorders = [ CallRecorder, InstructionRecorder ]
        self.check_events(self.func2,
                          recorders = recorders, must_include = self.MUST_INCLUDE_CI)

    def test_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.
    @staticmethod
    def func1():
        line1 = 1

    MUST_INCLUDE_LI = [
            ('instruction', 'func1', 2),
            ('line', 'func1', 2),
            ('instruction', 'func1', 4),
            ('instruction', 'func1', 6)]

    def test_line_then_instruction(self):
        recorders = [ LineRecorder, InstructionRecorder ]
        self.check_events(self.func1,
                          recorders = recorders, must_include = self.MUST_INCLUDE_LI)

    def test_instruction_then_line(self):
        recorders = [ InstructionRecorder, LineRecorder ]
        self.check_events(self.func1,
                          recorders = recorders, must_include = self.MUST_INCLUDE_LI)
    @staticmethod
    def func2():
        len(())

    MUST_INCLUDE_CI = [
            ('instruction', 'func2', 2),
            ('call', 'func2', sys.monitoring.MISSING),
            ('call', 'len', ()),
            ('instruction', 'func2', 12),
            ('instruction', 'func2', 14)]



    def test_call_then_instruction(self):
        recorders = [ CallRecorder, InstructionRecorder ]
        self.check_events(self.func2,
                          recorders = recorders, must_include = self.MUST_INCLUDE_CI)

    def test_instruction_then_call(self):
        recorders = [ InstructionRecorder, CallRecorder ]
        self.check_events(self.func2,
                          recorders = recorders, must_include = self.MUST_INCLUDE_CI)

cc @markshannon @sobolevn

@bedevere-bot bedevere-bot added awaiting core review tests Tests in the Lib/test dir labels Sep 8, 2023
@bedevere-bot bedevere-bot mentioned this pull request Sep 8, 2023
Closed
@hugovk hugovk mentioned this pull request Sep 8, 2023
Merged
@hugovk
Copy link
Member Author

hugovk commented Sep 18, 2023

(Resolved conflict.)

@markshannon
Copy link
Member

Thanks

@markshannon markshannon merged commit ea530f2 into python:main Oct 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

@AlexWaygood AlexWaygood assigned hugovk and unassigned markshannon Oct 12, 2023
@hugovk hugovk deleted the fix-duplicate-names-test_monitoring branch October 12, 2023 22:24
@bedevere-app
Copy link

bedevere-app bot commented Oct 15, 2023

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

@bedevere-app bedevere-app bot 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
@hugovk
[3.12] pythongh-106193: Rename and fix duplicated tests in `test_moni…
15ea3d5 
…toring` (pythonGH-109139)

(cherry picked from commit ea530f2)

Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
@hugovk hugovk mentioned this pull request Oct 15, 2023
Merged
hugovk added a commit that referenced this pull request Oct 15, 2023
@hugovk
[3.12] gh-106193: Rename and fix duplicated tests in test_monitoring (
9c39149 
GH-109139) (#110897)
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@hugovk @Glyphack
pythongh-106193: Rename and fix duplicated tests in test_monitoring (…
ace4b62 
…pythonGH-109139)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markshannon markshannon Awaiting requested review from markshannon

Assignees

@hugovk hugovk

Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

test_monitoring has duplicated tests
4 participants
@hugovk @markshannon @miss-islington @bedevere-bot