Skip to content

test: add unit tests for Variant, ReferenceMapping, Haplotype, and _parse_pop_freqs#27

Merged
ameynert merged 2 commits intomainfrom
am_15_test_remap_divref_models
Apr 20, 2026
Merged

test: add unit tests for Variant, ReferenceMapping, Haplotype, and _parse_pop_freqs#27
ameynert merged 2 commits intomainfrom
am_15_test_remap_divref_models

Conversation

@ameynert
Copy link
Copy Markdown
Collaborator

@ameynert ameynert commented Apr 17, 2026

Extends test_remap_divref.py with direct tests for the model methods and private helper that were previously only exercised indirectly via reference_mapping():

  • Variant.render(): SNP, insertion, deletion
  • Haplotype.parsed_variants(): single variant, multiple variants, caching (second call returns same list object)
  • Haplotype.contig(): returns chromosome of first variant
  • ReferenceMapping.variants_involved_str(): empty, single, multiple
  • _parse_pop_freqs(): all floats, mixed nulls, single null

Summary by CodeRabbit

Tests

  • Expanded test coverage for variant parsing, formatting, and helper functions
  • Added tests for variant rendering formats, haplotype parsing with caching verification, population frequency parsing, and error handling for malformed input

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 17, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e8af9465-fc42-4755-a16e-27784bf4ce27

📥 Commits

Reviewing files that changed from the base of the PR and between 0a3aef2 and 32993fe.

📒 Files selected for processing (1)
  • divref/tests/tools/test_remap_divref.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • divref/tests/tools/test_remap_divref.py

📝 Walkthrough

Walkthrough

This pull request adds comprehensive unit tests for the remap_divref module, covering functionality for variant rendering, haplotype parsing, reference mapping operations, and population frequency parsing. The test module docstring was updated and new test cases were added with appropriate imports.

Changes

Cohort / File(s) Summary
Test Module Updates
divref/tests/tools/test_remap_divref.py
Updated module docstring to reflect remap_divref coverage; added imports for pytest, Variant, and _parse_pop_freqs; added six unit test functions covering variant rendering, haplotype variant parsing and caching, contig extraction, reference mapping string formatting, population frequency parsing, and error handling for malformed variant input.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • feat: add remap_divref tool #12: Introduces the remap_divref tool with Variant, Haplotype, ReferenceMapping, and _parse_pop_freqs implementations that are directly tested by the new unit tests in this PR.

Poem

🐰 Whiskers twitch with testing pride,
Methods now have tests inside,
Variants parse and frequencies flow,
Coverage blooms—our code's aglow!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.56% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding unit tests for four key components (Variant, ReferenceMapping, Haplotype, _parse_pop_freqs), which directly matches the changeset that adds comprehensive test coverage for these entities.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch am_15_test_remap_divref_models

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
divref/tests/tools/test_remap_divref.py (1)

191-217: Add one explicit error-path test for parsing/contig edge inputs.

This block has strong happy-path coverage, but it should also lock behavior for malformed or empty variant strings (e.g., invalid delimiter count), especially since contig() depends on parsed output indexing.

🧪 Suggested test additions
+import pytest
+
+def test_parsed_variants_malformed_variant_raises() -> None:
+    hap = create_haplotype(variants="chr1:100:A", n_variants=1)
+    with pytest.raises(ValueError):
+        hap.parsed_variants()
+
+
+def test_contig_with_malformed_variant_raises() -> None:
+    hap = create_haplotype(variants="chr1:100:A", n_variants=1)
+    with pytest.raises(ValueError):
+        hap.contig()

As per coding guidelines, "**/*.py: New public functions require at least one happy-path test + one error case; bug fixes should include regression test".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@divref/tests/tools/test_remap_divref.py` around lines 191 - 217, Add a
failing-input unit test that verifies parsing and contig() handle malformed or
empty variant strings: use create_haplotype with variants="" and with a
malformed entry like "chr1:100:A" (wrong delimiter count) and assert
parsed_variants() raises the expected exception or returns an empty list (match
project behavior), and that contig() either raises the same error or returns
None/empty string accordingly; reference the parsed_variants() and contig()
methods to locate the parsing logic and ensure the test asserts the explicit
error-path contract expected by the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@divref/tests/tools/test_remap_divref.py`:
- Around line 191-217: Add a failing-input unit test that verifies parsing and
contig() handle malformed or empty variant strings: use create_haplotype with
variants="" and with a malformed entry like "chr1:100:A" (wrong delimiter count)
and assert parsed_variants() raises the expected exception or returns an empty
list (match project behavior), and that contig() either raises the same error or
returns None/empty string accordingly; reference the parsed_variants() and
contig() methods to locate the parsing logic and ensure the test asserts the
explicit error-path contract expected by the codebase.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d97de64f-d5ec-4df6-b55f-f88044aff736

📥 Commits

Reviewing files that changed from the base of the PR and between 57fd3ea and 0a3aef2.

📒 Files selected for processing (1)
  • divref/tests/tools/test_remap_divref.py

@ameynert ameynert force-pushed the am_11_remap_divref branch from 57fd3ea to a58f1dc Compare April 17, 2026 15:38
@ameynert ameynert force-pushed the am_15_test_remap_divref_models branch from 0a3aef2 to ff42a60 Compare April 17, 2026 15:44
@ameynert ameynert temporarily deployed to github-actions-snakemake-linting April 17, 2026 15:44 — with GitHub Actions Inactive
@ameynert ameynert force-pushed the am_11_remap_divref branch from a58f1dc to 3294ba9 Compare April 20, 2026 22:08
Base automatically changed from am_11_remap_divref to main April 20, 2026 22:10
ameynert and others added 2 commits April 20, 2026 15:11
…arse_pop_freqs

Extends test_remap_divref.py with direct tests for the model methods
and private helper that were previously only exercised indirectly via
reference_mapping():

- Variant.render(): SNP, insertion, deletion
- Haplotype.parsed_variants(): single variant, multiple variants,
  caching (second call returns same list object)
- Haplotype.contig(): returns chromosome of first variant
- ReferenceMapping.variants_involved_str(): empty, single, multiple
- _parse_pop_freqs(): all floats, mixed nulls, single null

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Add three tests covering ValueError propagation when parsed_variants()
receives an empty string or a variant token with fewer than 4 colon-
delimited fields, and that contig() surfaces the same error.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@ameynert ameynert force-pushed the am_15_test_remap_divref_models branch from ff42a60 to 32993fe Compare April 20, 2026 22:12
@ameynert ameynert temporarily deployed to github-actions-snakemake-linting April 20, 2026 22:12 — with GitHub Actions Inactive
@ameynert ameynert merged commit a52c416 into main Apr 20, 2026
4 checks passed
@ameynert ameynert deleted the am_15_test_remap_divref_models branch April 20, 2026 22:14
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