Skip to content

test(sqlite3): add uninstrument, error status, suppress, and no-op tests#4335

Merged
xrmx merged 6 commits into
open-telemetry:mainfrom
sophotechlabs:test/sqlite3-coverage-gaps
Apr 20, 2026
Merged

test(sqlite3): add uninstrument, error status, suppress, and no-op tests#4335
xrmx merged 6 commits into
open-telemetry:mainfrom
sophotechlabs:test/sqlite3-coverage-gaps

Conversation

@archy-rock3t-cloud

@archy-rock3t-cloud archy-rock3t-cloud commented Mar 15, 2026

Copy link
Copy Markdown
Contributor

Description

The sqlite3 instrumentation had only 3 tests covering basic execute/executemany/callproc. This adds the standard lifecycle tests that other dbapi-based instrumentations (psycopg2, mysql) already have: uninstrument, uninstrument_connection, error status on failed queries, suppress_instrumentation, and NoOpTracerProvider.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

tox -e py314-test-instrumentation-sqlite3 — all 8 tests pass (3 existing + 5 new).

Does This PR Require a Core Repo Change?

  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@JWinermaSplunk JWinermaSplunk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Few comments, also missing a CHANGELOG entry whenever you have a moment.

Comment thread instrumentation/opentelemetry-instrumentation-sqlite3/tests/test_sqlite3.py Outdated
Comment thread instrumentation/opentelemetry-instrumentation-sqlite3/tests/test_sqlite3.py Outdated
@herin049

Copy link
Copy Markdown
Contributor

LGTM, just needs a CHANGELOG.md entry.

@xrmx xrmx moved this to Approved PRs that need fixes in Python PR digest Mar 17, 2026
@archy-rock3t-cloud
archy-rock3t-cloud force-pushed the test/sqlite3-coverage-gaps branch from 500c19e to a9635ea Compare March 27, 2026 12:27
@archy-rock3t-cloud
archy-rock3t-cloud requested a review from a team as a code owner March 28, 2026 17:50
@archy-rock3t-cloud

Copy link
Copy Markdown
Contributor Author

@JWinermaSplunk @herin049 updated, please take a look, all comments have been addressed.

@archy-rock3t-cloud
archy-rock3t-cloud force-pushed the test/sqlite3-coverage-gaps branch from e52119a to 792e27d Compare March 30, 2026 13:40
@archy-rock3t-cloud

Copy link
Copy Markdown
Contributor Author

added additional tests

@tammy-baylis-swi tammy-baylis-swi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Much appreciated!

- Add _connect helper with addCleanup for proper resource teardown
- Rename test_uninstrument_connection to test_uninstrument_connection_with_instrument for consistency with psycopg2
- Use same query before and after uninstrumenting
- Clear memory exporter and assert zero spans after uninstrument
- Use sqlite3.OperationalError instead of generic Exception
- Add span.status.description assertion for error test
- Add CHANGELOG entry

Signed-off-by: Artem Muterko <[email protected]>
Signed-off-by: Artem Muterko <[email protected]>
Signed-off-by: Artem Muterko <[email protected]>
Signed-off-by: Artem Muterko <[email protected]>
@archy-rock3t-cloud
archy-rock3t-cloud force-pushed the test/sqlite3-coverage-gaps branch from 792e27d to 5b598a5 Compare April 6, 2026 12:55
@tammy-baylis-swi tammy-baylis-swi moved this from Approved PRs that need fixes to Approved PRs in Python PR digest Apr 13, 2026
@xrmx
xrmx enabled auto-merge (squash) April 20, 2026 13:13
@xrmx
xrmx merged commit 0158026 into open-telemetry:main Apr 20, 2026
750 of 751 checks passed
@github-project-automation github-project-automation Bot moved this from Approved PRs to Done in Python PR digest Apr 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants