Refactor seat_link_base_service_spec for public interface testing

Problem

The spec/services/utilization/seat_link_base_service_spec.rb spec has several tests that dive too deeply into implementation specifics rather than testing observable behavior. Here are the main concerns:

  1. Private method testing - The spec extensively tests private methods directly (:build_cloud_license, :license_by_license_key, :find_or_build_seat_link, etc.). These should be tested through public behavior instead.

  2. Attribute assignment verification - Tests like #save_seat_link verify exact attribute assignments (expect(seat_link.active_user_count).to eq(10)). This tests implementation details rather than the outcome. We should verify the returned seat_link

  3. Helper method tests - Methods like #billable_users_count, #max_count, #gitlab_version, #report_timestamp are simple accessors/transformers that don't need individual test coverage. They're tested implicitly through #execute.

  4. Mocking internal calls - Tests mock LicenseSeatLink.create_with and find_or_initialize_by calls, which are implementation details. The test should verify the seat link is created/found correctly, not how it's done.

  5. Abstract method testing - The #abstract_methods test uses .send() to call private methods, which violates the "no private method testing" rule.

Proposal

Focus tests on:

  • Public #execute behavior - Does it return the right result? Does it call the right tasks?
  • Observable outcomes - Is the seat link saved with correct data? Are errors logged appropriately?
  • Edge cases at the boundary - Invalid versions, missing data, save failures

Remove or consolidate:

  • Individual private method tests
  • Tests verifying exact attribute assignments (test through the saved/returned record instead)
  • Helper method tests that are just parameter extraction
  • The abstract methods test using .send()
Edited Jan 20, 2026 by Florian Jedelhauser
Assignee Loading
Time tracking Loading