Skip to content

Add Sothera dies-edict regression tests (#5255) - #5604

Closed
andriypolanski wants to merge 2 commits into
phase-rs:mainfrom
andriypolanski:fix/5255-sothera-supervoid-dies-edict
Closed

Add Sothera dies-edict regression tests (#5255)#5604
andriypolanski wants to merge 2 commits into
phase-rs:mainfrom
andriypolanski:fix/5255-sothera-supervoid-dies-edict

Conversation

@andriypolanski

Copy link
Copy Markdown
Contributor

Closes #5255

Summary

Test plan

  • issue_5255_sothera_dies_trigger_parses_opponent_scoped_exile_edict
  • issue_5255_sothera_dies_trigger_exiles_opponent_creature
  • sothera_each_opponent_exiles_own_creature (existing)
  • sothera_reanimates_only_linked_creature_with_two_counters (existing)

Verification

cargo fmt --all
cargo test -p engine issue_5255 -- --nocapture
cargo test -p engine sothera -- --nocapture

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@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.

Blocked — the test binary does not compile: this PR registers a module whose file it does not ship. Fix is one line; the linked-issue claim also needs correcting.

🔴 Blocker 1 — main.rs registers a module from your other branch (build break)

crates/engine/tests/integration/main.rs:781 adds:

mod issue_5263_chaos_warp;

but this PR ships only issue_5255_sothera_supervoid.rs. issue_5263_chaos_warp.rs lives in #5600, not here. CI says it plainly:

error[E0583]: file not found for module `issue_5263_chaos_warp`

That single stray line is why all four Rust checks fail (lint, coverage-gate, and both test shards) — the whole integration test binary fails to build, so nothing in it runs, including your Sothera test.

Fix: drop the mod issue_5263_chaos_warp; line from this PR's main.rs. Keep only mod issue_5255_sothera_supervoid;. It looks like this branch was cut from your #5600 branch and picked up its registration; rebasing onto a clean origin/main will also clear it.

🔴 Blocker 2 — Closes #5255 is not supported by the issue

The PR body says Closes #5255, which would auto-close that issue on merge. Two problems:

  1. The issue is internally inconsistent. #5255 is titled "Sothera, the Supervoid?" and its Summary describes a dies-edict trigger — but its "Oracle text (verified from card-data.json)" section is for a different card, The End (Cards: the end): "This spell costs {2} less to cast if your life total is 5 or less. Exile target creature or planeswalker…". The Discord extraction attached the wrong card, so the issue does not actually establish the Sothera behavior you are testing.
  2. Parser status: has_gaps on that issue — the mechanic is flagged as not fully parsed. A test asserting the fixed behavior is premature until the fix lands.

Fix: remove the closing keyword (use Refs #5255 if you want the backlink). The issue needs re-triage — its Oracle text does not match its title — and should not be silently closed by a test PR.

🟡 Non-blocking

Test placement is correct: crates/engine/tests/integration/ with a mod line, so no no_top_level_test_binaries violation. That part is right.

Recommendation: request-changes. Remove the stray mod issue_5263_chaos_warp;, rebase onto clean origin/main, and drop Closes #5255. Once the binary compiles, the Sothera test can actually be evaluated on its merits — right now it never runs.

@matthewevans matthewevans added the enhancement New feature or request label Jul 11, 2026
The phase-rs#5255 branch accidentally registered mod issue_5263_chaos_warp without
the module file, breaking integration test compilation in CI.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

Copy link
Copy Markdown

Parse changes introduced by this PR

✓ No card-parse changes detected.

@matthewevans

Copy link
Copy Markdown
Member

Thanks for the quick turnaround on the stray mod — that blocker is genuinely fixed, and CI is fully green on b49e289fb6 (13/13, real test shards).

I'm going to decline this one, though, and I want to be specific about why so it's useful rather than just a "no".

The coverage already exists on main. crates/engine/tests/integration/sothera_supervoid_edict_reanimate.rs (registered at main.rs:619) already covers this exact card, this exact Oracle text, and this exact behavior — il_each_opponent_exiles_own_creature drives the dies-edict through the real resolution path. Your own doc comment notes the new test falls back to "the parsed dies-edict body (same as sothera_supervoid_edict_reanimate)", so we'd be adding a second copy of a test we already run.

The one genuinely new idea is neutralized by its own fallback. Driving the trigger through process_triggers instead of hand-building the ability would be a real improvement in fidelity — that's a seam worth covering. But the test falls back to the parsed body when the trigger path doesn't fire, which means it can pass without ever exercising process_triggers. A test with a fallback can't tell you which path ran, so it can't fail for the reason you want it to. If you want to pursue that, a test that drives process_triggers and fails when the trigger doesn't collect would be genuinely valuable — that's the version I'd merge.

On issue selection — and this is the most useful thing I can give you. Two things to check before opening a regression-test PR:

There are ~19 open bug-labeled issues that are none of those things, and your track record on real engine work is excellent — #5584's ForEachCategory parameterization was the best architectural call I reviewed all week. That's where your time pays off most.

Closing this one; #5255 stays open for triage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sothera, the Supervoid? — Whenever a creature I control dies, an opponent should have a choice of creature they control…

2 participants