test(mysql): provide concrete DBAPI connection attributes in mocks #4116
Conversation
|
This PR only updates tests and does not change runtime behavior or public APIs. |
| cnx.database = "test" | ||
| cnx.host = "localhost" | ||
| cnx.port = 3306 | ||
| # Provide set values to avoid Magicbook attribute warnings |
There was a problem hiding this comment.
@xrmx yes i could think of them to avoid attribute type warnings in the tests. Without setting concrete primitive values, 'MagicMock' objects get propagated into span attributes like db.name, net.peer.name, and net.peer.port, which triggers noisy warnings.
If you had a different approach in mind , I’m happy to make changes.
There was a problem hiding this comment.
But cnx is not a mock, it's the real thing
There was a problem hiding this comment.
But cnx is not a mock, it's the real thing
Oh my bad i did not take it into account, i will update it asap thanks for review :)
|
|
||
|
|
||
| def make_mysql_connection_mock(): | ||
| cnx = mock.MagicMock() |
There was a problem hiding this comment.
You can pass attributes to your mock invocation, e.g. mock.MagicMock(database="test")
| cnx.host = "localhost" | ||
| cnx.port = 3306 | ||
|
|
||
| cursor = mock.MagicMock() |
There was a problem hiding this comment.
Since some of this code is open coded in the tests, maybe create an helper and use it there and in the tests that requires it?
There was a problem hiding this comment.
@xrmx gotcha I’ve updated the helper to pass attributes directly via MagicMock() and refactored the repeated setup into a small reusable helper to avoid duplication across tests.
Let me know if you'd prefer a different structure or naming here.
There was a problem hiding this comment.
I meant to call this helper in the tests where you are adding the very same thing but open coded. If you can't do that then try to split this helper in smaller pieces you can reuse in the tests.
There was a problem hiding this comment.
@xrmx Yep, I went with the helper approach you suggested - refactored the repeated MagicMock setup into a shared make_mysql_commenter_mocks helper and updated all SQL commenter tests to use it.
This cleans up a lot of duplication . Let me know if you'd like anything changed or tweaked
2 participants
Description
This PR updates the MySQL instrumentation integration tests to provide concrete DBAPI connection attributes (
host,port,database) in mocked connection and cursor objects.Previously, several tests emitted OpenTelemetry warnings due to
MagicMockvalues being used for attributes such asdb.name,net.peer.name, andnet.peer.port. These warnings were noisy but did not cause test failures.By setting realistic primitive values in the mocks, the tests now run without attribute type warnings, improving signal-to-noise ratio while keeping behavior unchanged.
Fixes #4114
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Ran MySQL instrumentation integration tests locally: tox -e py310-test-instrumentation-mysql-1
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.