Skip to content

test(engine): add Dina sacrifice-draw regression tests (#718)#5303

Merged
matthewevans merged 5 commits into
phase-rs:mainfrom
andriypolanski:test/issue-718-dina-sacrifice-draw
Jul 7, 2026
Merged

test(engine): add Dina sacrifice-draw regression tests (#718)#5303
matthewevans merged 5 commits into
phase-rs:mainfrom
andriypolanski:test/issue-718-dina-sacrifice-draw

Conversation

@andriypolanski

@andriypolanski andriypolanski commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes the loop on issue #718 by adding end-to-end integration tests for Dina, Essence Brewer's "Whenever you sacrifice a creature, draw a card" trigger. No engine changes — the runtime fixes were already on main:

  • PR #3165match_sacrificed LKI lookback for ceased-to-exist tokens (CR 603.10a)
  • PR #3733 — defer target selection after sacrifice costs for the activated ability (covered separately in greater_good_activation)

This PR adds the discriminating runtime tests proposed in the issue's implementation plan but never landed.

Oracle text

Whenever you sacrifice a creature, draw a card. This ability triggers only once each turn.
{2}, {T}, Sacrifice another creature: You gain X life and put X +1/+1 counters on target creature you control, where X is the sacrificed creature's power.

Changes

File Change
crates/engine/tests/integration/issue_718_dina_sacrifice_draw.rs Three integration tests driving the real activation pipeline
crates/engine/tests/integration/main.rs Register new integration module

Tests

Test Asserts
dina_draws_when_you_sacrifice_a_creature Sacrificing via Dina's activated ability draws 1 card, gains X life, and puts X +1/+1 counters (X = sacrificed power)
dina_draw_trigger_fires_only_once_per_turn A second sacrifice in the same turn does not draw again (CR 603.2h / OncePerTurn)
dina_draws_when_you_sacrifice_a_token_creature Sacrificing a token still triggers the draw via LKI after the token ceases to exist (CR 111.7)

All tests use GameScenario + AbilityActivation::resolve() — no hand-built trigger state.

Test plan

  • cargo test -p engine --test integration dina_draw
  • cargo fmt --all
  • test-engine Tilt resource green (if applicable)

Closes

Fixes #718

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request adds integration tests for Dina, Essence Brewer (issue #718) to verify her sacrifice-to-draw trigger, ensuring it triggers on creature sacrifice, respects the once-per-turn constraint, and correctly handles token sacrifices using last-known information. The feedback suggests querying the resolved outcome states instead of the runner's state directly to maintain consistency and avoid relying on in-place runner mutations.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread crates/engine/tests/integration/issue_718_dina_sacrifice_draw.rs
Comment thread crates/engine/tests/integration/issue_718_dina_sacrifice_draw.rs
@matthewevans matthewevans self-assigned this Jul 7, 2026
@matthewevans matthewevans added the test Add tests label Jul 7, 2026

@matthewevans matthewevans left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maintainer review: test-only regression coverage for #718, Gemini comments addressed, CR citations verified, and branch updated with origin/main.

@matthewevans matthewevans enabled auto-merge July 7, 2026 13:08
@matthewevans matthewevans removed their assignment Jul 7, 2026
@matthewevans matthewevans added this pull request to the merge queue Jul 7, 2026
Merged via the queue into phase-rs:main with commit ec764a2 Jul 7, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Add tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dina, Essence Brewer — The sacrifice trigger doesn't activate.

2 participants