Skip to content

test(byllm): deslop the test suite: one fake LLM, table-driven clusters, and per-PR rules #8930

Description

@kugesan1105

test(byllm): deslop the test suite: one fake LLM, table-driven clusters, and per-PR rules

Problem

The byLLM suite under jac/jaclang/byllm/tests/ is 244 tests in 6,308 lines plus 60 fixture programs. About 210 of those tests landed in 2026 across roughly 55 PRs, and only 2 PRs in that period consolidated anything. Every fix PR appends a sibling test and often a new fixture, so the suite grows at ~26 tests a month with no shared architecture.

What that produced:

  • Six independent ways to fake the LLM: module-level MockLLM outputs lists (43 fixtures), patch.object(model, "model_call_no_stream" / "model_call_with_stream") with hand-built SimpleNamespace responses, patch("litellm.completion"), monkeypatched dispatch_no_streaming on a MockLLM, ad-hoc _serve closures, and _FakeTC objects that bypass the model. Root cause: MockLLM only scripts the non-streaming path, so anything touching streaming, tool-call chunks, or usage has to invent its own fake.
  • Three fake MTRuntime scaffolds (_MockMTRun, make_fake_mt_run, and the seven-argument MTRuntime(...) constructor pasted 16 times), then re-wrapped by _mk_run, _mk_forced_run, _finish_env, _usage_from.
  • Five builders for the same OpenAI chat-completion response shape (_finish_env, _mk_forced_run, _tc_chunk/_text_chunks, _chunk/_frags/_plain, _response). _finish_env (PR fix(byllm): structured-output robustness for finish_tool arguments and json_object-downgrading backends #8246) and _mk_forced_run (PR fix(byllm): finish_tool-related calls stop shrinking the tools array (Anthropic cache-prefix fix) #8386, one week later) are near-verbatim copies.
  • In-file boilerplate: step_a and finish_tool defined 9 times in test_compaction.jac; capture_cb defined 8 times in test_telemetry_.jac; 45 tests in test_byllm.jac open with the identical five-line stdout-capture prelude; 10 fixtures assert inside the fixture and print a _PASS sentinel so the test checks only a string.
  • Sibling tests where a row was needed: api key resolution (9 tests), output conversion errors (7), litellm debug config (7), model pool (6 tests over 5 fixtures), typed retry (5), issue 7657 (4, named after the ticket), tools-array cache prefix (4 with two duplicate helper families).
  • Near-duplicate fixtures: react_max_iterations.jac vs react_max_iterations_finish_tool.jac (one output line differs), five model_pool_*.jac sharing one router mock, three logging_stream_*.jac sharing one skeleton.
  • No TESTING.md for byLLM and an empty fixtures/__init__.py, so each author starts from scratch in whichever file they touch.

The good patterns already exist in the repo and are not being reused: the positions row table in jac/tests/langserve/test_server.jac, the LanguageServerTestHelper object in jac/tests/langserve/server_test/test_lang_serve.jac, extend-by-assert in jac/tests/language/test_language.jac, parametrize() from jaclang.testing.test, and within byLLM itself test_usage_cost.jac (5 dispatch paths, 2 tests, one PATHS table and one runner).

Target architecture

One support annex, tests/support_tests.jac (not test_* so it is never collected), that owns every fake:

  • FakeLLM: a BaseLLM subclass that records every outgoing params dict and serves scripted replies on all five dispatch paths (dispatch_no_streaming, adispatch_no_streaming, dispatch_streaming, dispatch_streaming_with_tools, adispatch_streaming). Replies are declared as text, ToolCall, raw JSON, chunk stream, or an exception. This replaces the six faking seams. Where it makes sense, the same capability goes into the product's MockLLM so users get it too.
  • response(...), tool_call_reply(...), chunks(...): the single OpenAI-shape builders.
  • mk_run(...): the one MTRuntime factory with keyword overrides.
  • run_fixture(name) -> str: imports a fixture and returns captured stdout; fixtures stop printing sentinels and the test asserts.
  • capture_logs(...): the log-capture context used by the 7657 and debug tests.

Suite shape after the work: test_byllm.jac splits by subsystem (config_tests, output_tests, finish_tool_tests, streaming_tests, async_tests), each cluster is one table-driven test (a row list with a why label, or parametrize() when per-row reporting matters), and a regression is a new row or a new assert in the existing test for that behaviour.

Rules for every byLLM PR from now on (reviewers check these)

  • A regression test is a new row or assert in the existing test for that behaviour, not a sibling test named after the issue.
  • No new fake LLM, response builder, or MTRuntime constructor in a test file. Use support_tests.jac; extend it if a seam is missing.
  • No new fixture program when an inline def ... by llm(...) in the test body covers the scenario. A fixture is for multi-module or compile-time behaviour only.
  • Fixtures do not assert or print sentinels. The test asserts on returned values or captured output.
  • A new test file only for a new subsystem, never for a new PR.
  • PR description names which existing test each new row went into.

Phases

Phase 0 (P0, blocks everything else): the seam

Phase 1 (P0): collapse test_byllm.jac (114 tests, 2,438 lines)

Phase 2 (P1): fixtures (60 programs, 43 building their own MockLLM)

  • Merge react_max_iterations.jac and react_max_iterations_finish_tool.jac into one fixture with two entry functions.
  • Merge the five model_pool_*.jac fixtures into one with a shared router mock; 6 tests to 1 table.
  • Merge the three logging_stream_*.jac fixtures into one.
  • Convert single-function fixtures (llm_params_temp, api_key_verbose, plain_text_recovery, on_iteration_callback, conversation_param, and similar) to inline by llm defs in the test body.
  • Delete every fixture nothing references after the merges; add a guard test that fails on an unreferenced fixture.

Phase 3 (P1): the other files

  • test_compaction.jac: hoist step_a / finish_tool (9 copies) to module level; one compaction_llm(outputs, ctx_window) helper for the 10 MockLLM constructions; ctx_window priority (4 tests) to 1 table.
  • test_telemetry_.jac: one with_clean_registry() helper and one capture_cb factory (8 copies); callback registry tests (3) to 1; parent_invocation_id tests (2) fold into the record-fields table.
  • test_parallel.jac (42 tests): move _FakeTC, _FakeTool, _MockMTRun into support; serialize/default/timeout attribute tests (8) to 1 table; gate-logic tests (4) to 1 table; hint/annotation tests (5) to 1 table.
  • test_mtir_integration.jac: scope-name tests (3 trailing-letter cases) to 1 table; enum extraction tests (5) to 1 table.

Phase 4 (P2): boundaries

  • Reconcile parent_invocation_id coverage between byllm/tests/test_telemetry_.jac and scale/tests/misc/test_llm_telemetry.jac so each behaviour is tested once at the layer that owns it.
  • Decide the fate of validate_schema.jac (needs a live model and an API key; it is collected by directory runs). Either gate it behind an env var explicitly or move it to an examples/benchmarks folder.
  • Record before/after wall-clock for jac test -d jac/jaclang/byllm/tests in this issue.

Acceptance

File Now Target
test_byllm.jac 114 tests / 2,438 lines split into ~5 files, ~50 tests total
test_parallel.jac 42 ~25
test_mtir_integration.jac 29 ~20
test_compaction.jac 14 ~10
test_telemetry_.jac 13 ~8
fixtures 60 programs ~35
fake-LLM seams 6 1
MTRuntime constructors in tests 16 inline + 4 wrappers 1

Coverage must not drop: every deleted test maps to a row or assert in the PR that deletes it, and the PR description lists the mapping.

Progress

Non-goals

  • No behaviour changes in jaclang.byllm beyond extending MockLLM.
  • No rewrite of validate_schema.jac content; only how it is gated.
  • No changes to the scale telemetry store tests beyond the overlap in Phase 4.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions