Summary
Several independent test-quality improvements: tighten assertions/structure, and add a shared helper that centralizes setup. Items 1–4 can be done à la carte; item 5 (the helper) is the foundational structural change.
Directive
- Magic wire strings → constants:
tests/relay/call_test.rb, tests/relay/message_test.rb already require the relay constants but assert raw strings ('calling.answer', 'answered', 'queued'/'sent'/'delivered'). Use the defined constants.
- De-dup skill lists:
tests/skills_test.rb and tests/registry_test.rb repeat an identical ~18-element skill list — have both consume one shared constant. Leave the independent copy in tests/skill_name_test.rb (it's a deliberate cross-check).
- Strengthen weak assertions: replace presence-only checks (e.g.
assert tools[0].key?(:datamap), assert(result.key?('sid') || result.key?('name'))) with exact value/shape assertions (tests/skills/weather_api_skill_test.rb, tests/rest/compat_calls_streams_mock_test.rb).
- Split inline multi-path tests: one method exercises several input paths inline (
tests/skills/web_search_skill_test.rb, weather_api_skill_test.rb, datasphere_skill_test.rb) — split so a failure pinpoints the path.
- Add a shared
tests/test_helper.rb: there's no central helper, so global-state reset (skill registry, ENV) and the WEBrick/ephemeral-port + skill-fixture setup are duplicated across tests/cli_test.rb, tests/per_skill_parity_test.rb, and the skill tests — leaving an ENV/registry leakage window. Centralize global-state reset and the shared fixtures in tests/test_helper.rb; have test files require it.
Guardrails
The test_helper refactor (item 5) must not change what any test asserts — only where setup lives. Items 1 & 3 intentionally change assertions (to be stricter/exact).
Acceptance
Verify
rake test; run twice to check for order dependence.
Summary
Several independent test-quality improvements: tighten assertions/structure, and add a shared helper that centralizes setup. Items 1–4 can be done à la carte; item 5 (the helper) is the foundational structural change.
Directive
tests/relay/call_test.rb,tests/relay/message_test.rbalready require the relay constants but assert raw strings ('calling.answer','answered','queued'/'sent'/'delivered'). Use the defined constants.tests/skills_test.rbandtests/registry_test.rbrepeat an identical ~18-element skill list — have both consume one shared constant. Leave the independent copy intests/skill_name_test.rb(it's a deliberate cross-check).assert tools[0].key?(:datamap),assert(result.key?('sid') || result.key?('name'))) with exact value/shape assertions (tests/skills/weather_api_skill_test.rb,tests/rest/compat_calls_streams_mock_test.rb).tests/skills/web_search_skill_test.rb,weather_api_skill_test.rb,datasphere_skill_test.rb) — split so a failure pinpoints the path.tests/test_helper.rb: there's no central helper, so global-state reset (skill registry, ENV) and the WEBrick/ephemeral-port + skill-fixture setup are duplicated acrosstests/cli_test.rb,tests/per_skill_parity_test.rb, and the skill tests — leaving an ENV/registry leakage window. Centralize global-state reset and the shared fixtures intests/test_helper.rb; have test files require it.Guardrails
The
test_helperrefactor (item 5) must not change what any test asserts — only where setup lives. Items 1 & 3 intentionally change assertions (to be stricter/exact).Acceptance
tests/test_helper.rbexists; duplicated setup removedrake testand a shuffled run)Verify
rake test; run twice to check for order dependence.