Skip to content

[Test Improver] test: primitives models 78% → 100% coverage (Skill, conflict detection)#240

Merged
danielmeppiel merged 3 commits intomainfrom
test-assist/primitives-model-coverage-1773191453-21e144de9777f273
Mar 12, 2026
Merged

[Test Improver] test: primitives models 78% → 100% coverage (Skill, conflict detection)#240
danielmeppiel merged 3 commits intomainfrom
test-assist/primitives-model-coverage-1773191453-21e144de9777f273

Conversation

@danielmeppiel
Copy link
Collaborator

🤖 This PR was created by Test Improver, an automated AI assistant focused on improving test coverage.

Goal and Rationale

src/apm_cli/primitives/models.py had 78% coverage, with key logic paths completely untested:

  • Skill.validate() — no tests at all for the fourth APM primitive type
  • PrimitiveCollection conflict detection — the priority rules ("local wins over dependency") had zero test coverage; this is critical correctness logic
  • PrimitiveConflict.__str__() — untested string representation
  • Instruction.validate() branches — missing-description and empty-content paths

The conflict detection logic is particularly important: it determines which version of a primitive "wins" when the same name appears in both local files and installed dependencies. A bug here would silently corrupt users' primitive collections.

Approach

Added 17 focused unit tests to tests/unit/primitives/test_primitives.py:

  1. Skill.validate() — valid skill, missing name, missing description, empty content, all errors together
  2. Instruction.validate() — missing description branch, empty content branch, multiple errors
  3. PrimitiveConflict.__str__() — string representation with multiple losing sources
  4. PrimitiveCollection conflict detection:
    • Local wins over dependency (replace path in _add_with_conflict_detection)
    • Local not replaced when dependency arrives later (keep path)
    • First dependency beats second dependency (graceful tie-break)
  5. PrimitiveCollection query methodshas_conflicts(), get_conflicts_by_type(), get_primitives_by_source()
  6. Add Skill to collection — verifies the isinstance(primitive, Skill) path in add_primitive()

Coverage Impact

File Before After
src/apm_cli/primitives/models.py 78% (32 uncovered lines) 100%
Overall 55% 55% (+32 lines covered)

Test Status

1445 passed, 1 failed (pre-existing: test_install_command.py ANSI escape codes — unrelated to this PR)

Reproducibility

pip install uv --break-system-packages
python3 -m uv sync --extra dev
python3 -m uv run pytest tests/unit/primitives/test_primitives.py -v
python3 -m uv run pytest tests/unit/ --cov=apm_cli/primitives --cov-report=term-missing

Generated by Daily Test Improver ·

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/daily-test-improver.md@b87234850bf9664d198f28a02df0f937d0447295

… source filtering

- Add Skill.validate() tests (missing name, description, empty content)
- Add Instruction.validate() missing-description and empty-content branches
- Add PrimitiveConflict.__str__() test
- Add PrimitiveCollection conflict-detection tests:
  - local wins over dependency (replace)
  - local not replaced by later dependency (keep)
  - first dependency wins over second dependency
- Add PrimitiveCollection.has_conflicts(), get_conflicts_by_type(), get_primitives_by_source() tests
- Import Skill and PrimitiveConflict in test module

primitives/models.py: 78% -> 100%
Total: 1428 -> 1445 passing tests

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel danielmeppiel marked this pull request as ready for review March 12, 2026 21:06
Copilot AI review requested due to automatic review settings March 12, 2026 21:06
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Expands the primitives unit test suite to cover newly introduced primitive types and behaviors (notably Skill and primitive conflict handling), while also applying minor formatting/consistency tweaks.

Changes:

  • Add unit tests for Skill validation and PrimitiveConflict string formatting.
  • Add unit tests for PrimitiveCollection conflict-resolution behavior and new helpers (has_conflicts, filtering by type/source).
  • Apply small formatting cleanups (imports, commas, line wrapping, quoting) across the test file.

def test_primitive_collection_add_unknown_type_raises(self):
"""Test that adding an unknown type raises ValueError."""
collection = PrimitiveCollection()
with self.assertRaises((ValueError, AttributeError)):
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test currently accepts both ValueError and AttributeError when adding an unknown primitive. PrimitiveCollection.add_primitive() explicitly raises ValueError for unknown types, so allowing AttributeError could mask an unintended regression (e.g., an internal attribute access bug). Tighten the assertion to expect only ValueError (and optionally match the message).

Suggested change
with self.assertRaises((ValueError, AttributeError)):
with self.assertRaises(ValueError):

Copilot uses AI. Check for mistakes.
@danielmeppiel danielmeppiel merged commit eedc51f into main Mar 12, 2026
5 checks passed
@danielmeppiel danielmeppiel deleted the test-assist/primitives-model-coverage-1773191453-21e144de9777f273 branch March 12, 2026 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants