Follow-up from "Secrets permissions graphql improvements"

The following discussion from !214715 (merged) should be addressed:

Duo's explanation:

The rspec:undercoverage job is failing because new or modified code lacks sufficient test coverage. The job uses the undercover gem to detect methods that don't have adequate branch coverage.

Why it's failing

The job found 6 methods with incomplete branch coverage:

  1. permissions_to_actions - 50% coverage (missing branches in conditionals)
  2. actions_to_permissions_string - 88.89% coverage (missing branch)
  3. to_capabilities - 88.89% coverage (missing branch)
  4. set_actions_from_capabilities - 40% coverage (missing branches)
  5. valid_principal? - 75% coverage (missing branch)
  6. Block in list_service_helpers.rb - 66.67% coverage (missing branch)

How to fix it

You need to add tests that cover the missing branches (the red 1/2 indicators). Specifically:

1. For permissions_to_actions (line 78-80)

actions << 'read' if perms_set.include?('read')      # Missing: when 'read' is NOT included
actions << 'write' if perms_set.include?('create') || perms_set.include?('update')  # Missing: when neither is included
actions << 'delete' if perms_set.include?('delete')  # Missing: when 'delete' is NOT included

Add tests for:

  • When permissions don't include 'read'
  • When permissions don't include 'create' or 'update'
  • When permissions don't include 'delete'

2. For actions_to_permissions_string (line 35)

return '' if actions.blank?  # Missing: when actions is NOT blank

Add test for: When actions is not blank (already covered, but ensure edge cases)

3. For set_actions_from_capabilities (lines 93, 95, 99)

actions << ACTIONS[:read] if capabilities.include?(CAPABILITIES[:read])  # Missing: when NOT included
if capabilities.include?(CAPABILITIES[:create]) && capabilities.include?(CAPABILITIES[:update])  # Missing: when condition is false
actions << ACTIONS[:delete] if capabilities.include?(CAPABILITIES[:delete])  # Missing: when NOT included

Add tests for:

  • Capabilities without 'read'
  • Capabilities without both 'create' and 'update'
  • Capabilities without 'delete'

4. For valid_principal? (line 35)

return false if principal_id.blank? || principal_type.blank?  # Missing: when both are present

Add test for: When both principal_id and principal_type are present

5. For the block in list_service_helpers.rb (line 42)

if SecretsManagement::BaseSecretsPermission::VALID_CAPABILITIES.include?(capability)  # Missing: when NOT included

Add test for: When capability is not in VALID_CAPABILITIES

Quick fix approach

Add specs that exercise these code paths. For example, in your mutation/service specs, add test cases like:

# Test with empty permissions
it 'handles empty permissions' do
  # ...
end

# Test with invalid capabilities
it 'filters out invalid capabilities' do
  # ...
end

# Test with missing principal
it 'validates principal presence' do
  # ...
end

The undercoverage check ensures that all new/modified code paths are tested, preventing untested logic from being merged.

Edited by 🤖 GitLab Bot 🤖