Skip to content

test(logger): remove redundant tests and parametrize duplicates in test_logger.py - #830

Merged
Abhijeet Prasad (AbhiPrasad) merged 9 commits into
mainfrom
ref/test-logger-cleanup
Sep 29, 2026
Merged

test(logger): remove redundant tests and parametrize duplicates in test_logger.py#830
Abhijeet Prasad (AbhiPrasad) merged 9 commits into
mainfrom
ref/test-logger-cleanup

Conversation

@AbhiPrasad

@AbhiPrasad Abhijeet Prasad (AbhiPrasad) commented Sep 29, 2026 •
edited by starfolkai Bot
Loading

Copy link
Copy Markdown
Member

Summary

Cleans up py/src/braintrust/test_logger.py by removing redundant and near-empty tests and parametrizing near-duplicates. Test-only change; no SDK code is modified.

  • 4,999 → 3,436 lines, 223 → 189 tests in test_logger.py (one test moved to test_gitutil.py).
  • Each commit is one self-contained batch and can be reviewed on its own.

What changed

  • Removed no-op or duplicated tests, for example:
    • init dataset-id tests that never called init()
    • JSONAttachment tests that only checked objects they had just built
    • span.log binary/bad-key tests that asserted only that a key exists
    • attachment, deep-copy, id_gen and span_components tests already covered in test_bt_json.py, test_id_gen.py or test_span_components.py
  • Parametrized near-duplicates:
    • generator MAX_GENERATOR_ITEMS limits (sync/async × 3/0/-1)
    • span export format (9 tests → 1)
    • model_dump metadata across 7 logging APIs
    • masking across logger, experiment, child span and dataset: one shared masking function replaces 4 hand-written ones
    • prompt/parameters version-over-environment
    • strict-mode templates
    • bt-eval BTQL merge
    • atexit, proxy_conn
    • emit_log str.format/t-string template rendering
  • Dissolved the TestLogger grab-bag into plain pytest functions.
  • Replaced manual os.environ save/restore (and a hand-built pytest.MonkeyPatch()) with the monkeypatch fixture.
  • Moved test_get_repo_info_without_settings_returns_none to test_gitutil.py.

Assertions that got stricter

  • Generator limit 0 now also checks that no truncation warning is logged. The old tests missed a regression there.
  • The inferred child-span name is now checked exactly (user_fn:user_module.py:2) by calling start_span() from a module that isn't under braintrust.*. Before, the test only checked that the name wasn't empty.
  • The experiment span-link test checks the full URL instead of three substrings.
  • Template-rendering cases check the full metadata, the log level, and that no error field is set.
  • The masking test checks that no unmasked value appears anywhere in the logged rows.
  • New case: BRAINTRUST_OTEL_COMPAT takes priority over BRAINTRUST_LEGACY_IDS for the export format.

Test plan

  • nox -s test_core: 926 passed, 69 skipped, 12 xfailed (main: 955 passed). Outside test_logger.py, the only difference is the test moved into test_gitutil.py.
  • test_logger.py passes in 5 shuffled orders, and on Python 3.11 (t-string cases skip) as well as 3.14.
  • Mutation checks: for each rewritten area (generator limits, export selection, masking, version precedence, BTQL merge, template rendering, inferred span names), I broke the code under test in logger.py and confirmed the new tests fail.
  • ruff, pre-commit hooks, pylint --errors-only.

🤖 Generated with Claude Code

Co-authored by StarfolkAI (@starfolkai)[bot]

@AbhiPrasad @claude
test(logger): remove redundant and no-op tests
4e2adb0 
Drop tests in test_logger.py that either assert nothing meaningful or are
strict subsets of other tests (in this file, test_bt_json.py, test_id_gen.py,
or test_span_components.py):

- init dataset-id tests that never call init()
- JSONAttachment tests that only check objects they just constructed
- span.log binary/bad-key/large-document tests with near-empty assertions
- duplicated attachment extraction, deep-copy identity, and upload-tracking tests
- duplicate explicit-parent, otel flush, and dataset batch-size tests
- async/thread tests that call span.link() directly, bypassing context

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@AbhiPrasad @claude
test(logger): parametrize traced generator limit and async function t…
aabff5a 
…ests

- Collapse six sync/async BRAINTRUST_MAX_GENERATOR_ITEMS tests (limit 3, 0,
  -1) into one parametrized test using monkeypatch instead of manual env
  save/restore. The zero-limit case now also asserts no truncation warning,
  which the old tests missed.
- Split the invalid-max-items test by sync/async via the same helper.
- Parametrize test_traced_async_function over @Traced, @Traced(), and
  @Traced(name=...) instead of repeating the body three times.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@AbhiPrasad @claude
test(logger): consolidate span export format and otel ID tests
d9f419c 
- Replace nine _get_exporter/Experiment.export/Logger.export tests with one
  parametrized test over the ID-mode env vars, and add the case where
  BRAINTRUST_OTEL_COMPAT overrides BRAINTRUST_LEGACY_IDS.
- Fold the flush_otel no-callback test into test_register_otel_flush_callback.
- Use monkeypatch in reset_id_generator_state and the otel ID tests instead of
  manual os.environ save/restore; drop leftover debug prints.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@AbhiPrasad @claude
test(logger): parametrize metadata, serialization, and masking tests
1d47a89 
- Replace seven model_dump metadata tests (span/logger/experiment log,
  logger/experiment log_feedback, dataset insert/update) with one
  parametrized test.
- Merge the simple and nested circular-reference tests, and the NaN and
  Infinity tests, into single tests.
- Replace four masking tests, each with its own hand-rolled recursive masking
  function (and leftover debug prints), with one parametrized test over
  logger, experiment, child span, and dataset sharing one masking function.
  It also checks that no unmasked value appears anywhere in the logged rows.
- Drop the stale "This test currently FAILS" docstring.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@AbhiPrasad @claude
test(logger): dissolve TestLogger grab-bag and parametrize prompt tests
f1e137d 
- Convert the TestLogger TestCase methods to plain pytest functions.
- Replace four load_prompt/load_parameters "version over environment" tests,
  each inlining a full response body, with one parametrized test that reuses
  _prompt_response/_parameters_response.
- Collapse strict-mode template tests into present/missing parametrized
  tests sharing a single _test_prompt helper (which the structured-output
  test now uses too), and drop the render_mustache tests they subsume.
- Merge the two transient-error cache fallback tests and assert the second
  request actually reached the server.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@AbhiPrasad @claude
test(logger): parametrize init/state tests and use monkeypatch for env
f5994e4 
- Replace the atexit enable/disable TestCase methods with one parametrized
  test, adding the explicit "false" case.
- Collapse four bt-eval internal BTQL merge tests into one parametrized
  test using the monkeypatch fixture instead of a hand-built MonkeyPatch,
  and drop dead app_conn setup from the fetch forwarding test.
- Parametrize the two proxy_conn base URL tests.
- Use monkeypatch in the logged-out span link env var tests, and assert the
  full experiment link instead of three substrings.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@AbhiPrasad @claude
test(logger): trim span name and log level overlap, tidy leftovers
48b04a5 
- Merge span-name-matches-logged-data into the explicit-name test, drop the
  current_span().name test covered by the @Traced name test, and assert the
  inferred child span name's "func:file.py:line" format instead of non-empty.
- Rename the noop span name test to match what it asserts.
- Fold the emit_log log-level test into test_logger_log_level_helpers.
- Rename test_nested_spans_with_export to describe the login regression it
  covers, and drop redundant local imports and unused variables.
- Move the get_repo_info(None) test to test_gitutil.py.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@AbhiPrasad @claude
test(logger): consolidate emit_log template and t-string tests
26c5965 
- Fold test_logger_emit_log_enqueues_single_row into
  test_logger_emit_log_without_active_span (raw enqueue count and _is_merge).
- Replace four str.format template tests and two t-string rendering tests
  with one parametrized test over level, message, and parameters, gating the
  t-string cases per-param on Python 3.14. Every case now asserts the full
  metadata, the log level, and that no span error field is set.
- Add a small _t_string helper so t-string tests don't repeat
  importlib/templatelib boilerplate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@AbhiPrasad @claude
test(logger): simplify consolidated tests
cc1c498 
- Build test prompts with Prompt.from_prompt_data instead of a hand-rolled
  PromptSchema.
- Test the inferred child span name by calling start_span from a
  user-named module, asserting the exact "user_fn:user_module.py:2" name
  instead of matching whatever pytest frame happens to be the caller.
- Pass @Traced decorators and generator consumers as parametrize values
  instead of lambdas and string dispatch; add an expected_id column rather
  than deriving the id from the response shape.
- Reduce reset_id_generator_state to the env clearing it actually does (the
  autouse conftest fixture already replaces the cached state), use it in the
  export format test, and drop checks already covered by test_id_gen.py,
  test_env.py, and redundant parametrize rows.
- Remove a stray comment left by the deleted dataset-id tests and rename a
  misleading masking test column.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 29, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-29T15:00:49.298006Z cc1c498 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@AbhiPrasad
Abhijeet Prasad (AbhiPrasad) merged commit 9955fe4 into main Sep 29, 2026
83 checks passed
@AbhiPrasad
Abhijeet Prasad (AbhiPrasad) deleted the ref/test-logger-cleanup branch September 29, 2026 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@AbhiPrasad