Skip to content

Resolve duplicate test name in test_deal_entry.py (ar-r82f.19)#96

Merged
atc964 merged 1 commit into
mainfrom
fix/test-deal-entry-duplicate
May 28, 2026
Merged

Resolve duplicate test name in test_deal_entry.py (ar-r82f.19)#96
atc964 merged 1 commit into
mainfrom
fix/test-deal-entry-duplicate

Conversation

@atc964
Copy link
Copy Markdown
Collaborator

@atc964 atc964 commented May 28, 2026

Summary

Resolves bead ar-r82f.19. The file tests/unit/test_deal_entry.py had two functions named test_deal_data_includes_v2_fields_as_top_level_keys (lines 438 and 481). Python silently shadowed the first definition with the second, meaning the line-438 test never executed. PR #95's lint sweep masked the F811 warning with # noqa: F811 to keep its scope pure; this PR fixes the real bug.

What changed

  • Deleted the redundant duplicate test at line 481.
  • Removed the # noqa: F811 marker on the same line.

The two tests were functional duplicates:

  • Both imported the same ManualDealEntry / create_manual_deal from ad_buyer.tools.deal_library.deal_entry.
  • Both built a ManualDealEntry with the same v2 fields (only the display_name and description string values differed).
  • Both asserted that data["seller_org"], data["seller_domain"], data["seller_type"], data["buyer_org"], data["buyer_id"], data["price_model"], data["fixed_price_cpm"], data["bid_floor_cpm"], data["currency"], data["media_type"], data["description"] matched the supplied entry.

The retained test (line 438) is the canonical/newer version and is strictly stronger: it adds assert "metadata" not in data, which the deleted variant did not check. git blame confirmed line 438 was added in commit 633d813 (2026-03-25 14:07) after the now-removed variant in commit 0bf2992 (2026-03-25 13:43); the newer test was the intended replacement that simply never deleted its predecessor.

Test plan

  • python -c "import ast; ast.parse(open('tests/unit/test_deal_entry.py').read())" -- syntax OK
  • Duplicate name check: grep "^def test_\|^ def test_" tests/unit/test_deal_entry.py | sort | uniq -c -- no duplicates
  • ruff check tests/unit/test_deal_entry.py -- all checks passed
  • CI test job runs and the previously-shadowed test now executes (look for test_deal_data_includes_v2_fields_as_top_level_keys in pytest output)
  • No new lint warnings

bead: ar-r82f.19

Delete redundant duplicate of test_deal_data_includes_v2_fields_as_top_level_keys
at line 481. The function was a functional duplicate of the test at line 438
(same imports, fields, and assertions). Python silently shadowed the earlier
definition, so the first test never ran. Removed the noqa: F811 marker that
masked the underlying issue.

The retained test (formerly line 438) is more comprehensive, including the
additional `assert "metadata" not in data` check.

bead: ar-r82f.19

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@atc964 atc964 merged commit b461aed into main May 28, 2026
2 of 3 checks passed
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