Skip to content

test: cover character builders, roll check helper and http client - #114

Open
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1786669119-builder-coverage-specs
Open

test: cover character builders, roll check helper and http client#114
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1786669119-builder-coverage-specs

Conversation

@devin-ai-integration

Copy link
Copy Markdown

Summary

app/builders was the largest app directory with no specs of its own — its coverage came incidentally from create-character command specs, so the dispatcher fallbacks and most concrete builders were unexercised (dnd2024_character/background_builder.rb and tlc_character/background_builder.rb sat at 54.5%). This adds specs for the builder layer plus the two other lowest-covered app files, app/helpers/roll_check_helper.rb (33.3%) and app/lib/http_service/client.rb (42.1%). No production code changed.

Coverage: 94.49% → 95.29%, examples 793 → 1376. All targeted files are now 100% except http_service/client.rb, whose remaining gap is build_connection (never used in tests by design).

Three layers of builder specs:

  1. BaseBuilder specs pin the initial sheet each provider produces — notably the differences that are easy to break: TLC starts at START_LEVEL = 3 and seeds classes/hit_dice with 3 while dnd2024/dnd5 start at 1, dnd2024/TLC use a Hash for selected_skills while dnd5 uses an Array, and only dnd2024/TLC set guide_step.

  2. Dispatcher specs (class/species/background/legacies/race/subrace) cover both the constantize path and the rescue NameError => DummyBuilder fallback, which is what the low coverage was hiding. The success paths loop over the config rather than a hardcoded list, so a newly configured class/background/race is asserted automatically:

Dnd2024::Character.classes_info.each_key do |class_slug|
  it 'fills the hit dice slot from HIT_DICES' do
    expect(build[:hit_dice][Dnd2024::Character::HIT_DICES[class_slug]]).to eq(1)
  end
end

The TLC class spec derives which slugs have no TLC builder instead of hardcoding them, so adding one flips the expectation from "abilities stay unset" to the full assertion rather than leaving a stale skip behind.

  1. spec/builders/builder_contract_spec.rb sweeps all ~110 concrete builders (globbed off disk, minus the DB-backed CustomBuilders) against invariants the create commands rely on: the builder returns the sheet it was handed, the collections it appends to are duplicate-free and idempotent (a missing .uniq surfaces on a sheet as a doubled proficiency), and health/abilities are well formed when assigned.

HttpService::Client is exercised over Faraday's test adapter, asserting the request shape each verb builds (JSON body vs. URI.encode_www_form, params/headers pass-through), the success/nil/{ success: false } return contracts, and that the 'please stub request in test env' guard fires for every verb when no test adapter is wired.

Pre-existing failures on main are unaffected and unrelated: two javascript_include_tag 'application' asset-pipeline failures and spec/javascript_spec.rb (no spec/javascript/*.test.js in this branch).

Link to Devin session: https://app.devin.ai/sessions/e58270d7a3354edcba46a1c4146a6d2b
Requested by: @zacgoodwin

The app/builders tree had no specs of its own; coverage came only incidentally from create-character command specs, leaving the dispatchers' DummyBuilder fallbacks and most concrete builders unexercised.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@zacgoodwin zacgoodwin self-assigned this Aug 14, 2026
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant