Skip to content

test: assert reference rows have the expected number of values - #89

Closed
kilo-code-bot[bot] wants to merge 1 commit into
masterfrom
fix/test-length-check
Closed

test: assert reference rows have the expected number of values#89
kilo-code-bot[bot] wants to merge 1 commit into
masterfrom
fix/test-length-check

Conversation

@kilo-code-bot

@kilo-code-bot kilo-code-bot Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

What this PR changes

test_example in tests/test_examples.py truncated each computed row to the reference length (table[method][:len(expected)]), silently ignoring any extra columns a script might start producing. It now asserts the row length matches the reference before comparing values.

Why

CodeRabbit flagged the truncating comparison in the earlier version of this test; that code was rewritten, but the truncation survived in test_example. This closes the still-valid part: an added column (e.g. a new n_cands/n_voters/condition) now fails the test instead of being masked.

Tests

The example tests themselves (run with pytest -m slow); the length assertion is a no-op for the current scripts since every reference row already matches its output length.

Summary by CodeRabbit

  • Bug Fixes
    • Improved example validation to detect empty output tables.
    • Ensured generated rows match the expected number of values exactly.
    • Updated comparisons to validate complete result rows.

@what-the-diff

what-the-diff Bot commented Aug 4, 2026

Copy link
Copy Markdown

PR Summary

  • Enhanced Error Message for Assertion
    The message displayed when the lengths of parameters 'got' and 'expected' are validated for equality has been improved to be more informative.

  • Enhanced Assertion for table[method] and expected
    Altered the checks to verify that the lengths of table[method] and expected are identical, this change will make error reports more meaningful if they are not matching.

  • Adjusted Use of np.testing.assert_allclose
    Modified the call of np.testing.assert_allclose method to use table[method] directly in place of its sliced version, for more accurate and efficient testing.

test_example truncated each computed row to the reference length
(table[method][:len(expected)]), silently ignoring any extra columns a
script might start producing. Assert the row lengths match before
comparing values, so an added column fails the test instead of being
masked.

Also assert each script's table is non-empty, so a figure script that
produces no rows fails instead of passing silently.

Addresses the still-valid parts of CodeRabbit's review comments on the
earlier version of these tests.
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 71b94be7-a50a-4de5-bf8a-7792e9994433

📥 Commits

Reviewing files that changed from the base of the PR and between 6e08e9e and 0a2cfac.

📒 Files selected for processing (1)
  • tests/test_examples.py

📝 Walkthrough

Walkthrough

The example test now rejects empty tables, requires exact row lengths, and compares complete result rows with expected values.

Changes

Example validation

Layer / File(s) Summary
Exact result row validation
tests/test_examples.py
test_example rejects empty tables, enforces exact row lengths, and compares complete result rows.

Estimated code review effort: 2 (Simple) | ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: validating that computed rows match the expected number of values.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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 fix/test-length-check

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

@kilo-code-bot
kilo-code-bot Bot force-pushed the fix/test-length-check branch from 55259ac to 0a2cfac Compare August 4, 2026 18:28
@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.37%. Comparing base (6e08e9e) to head (0a2cfac).

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #89   +/-   ##
=======================================
  Coverage   96.37%   96.37%           
=======================================
  Files          17       17           
  Lines         496      496           
=======================================
  Hits          478      478           
  Misses         18       18           
Flag Coverage Δ
no-numba 95.76% <ø> (ø)
numba 88.10% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kilo-code-bot

kilo-code-bot Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by this PR (the length assertion is folded in).

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.

0 participants