Fix ASSUMPTIONS not working

[DizzyEggg]

Fixes #3043

Example output when ASSUME fails in ASSUMPTION block:

[0] Multi hit Moves hit the maximum amount with Skill Link: ASSUMPTION_FAIL
test/battle/move_effect/multi_hit.c:6: ASSUME failed
[0] Multi hit Moves hit twice 35 Percent of the time: ASSUMPTION_FAIL
[0] Multi hit Moves hit thrice 35 Percent of the time: ASSUMPTION_FAIL
[0] Multi hit Moves hit four times 35 Percent of the time: ASSUMPTION_FAIL
[0] Multi hit Moves hit four times 35 Percent of the time: ASSUMPTION_FAIL
[0] Multi hit Moves hit at least four times with Loaded Dice: ASSUMPTION_FAIL
[0] Multi hit Moves hit five times 50 Percent of the time with Loaded Dice: ASSUMPTION_FAIL
- Tests PASSED:        0
- ASSUMPTIONS_FAILED:  7
- Tests TOTAL:         7

[mrgriffin]

Ah, I think I see what I messed up—this code looks like it was expecting to return TRUE if the test should run on this runner, and FALSE if not; but somewhere along the way it changed to returning the process to run on instead.

The reason why you'd want to run the assumptions on every runner is that the tests which are covered by the assumptions run on an arbitrary set of runners. So I wonder if this should say return gTestRunnerI; so that the test runs on all the runners (at zero cost, because we've skipped estimateCost on line 98, which is also important for keeping all the processCosts in sync).

I think atm if we have no tests except:

• an ASSUMPTIONS block which fails
• two tests that rely on that block

Then if we make check -j2 the second test will run even though it shouldn't because ASSUMPTIONS and the first test will run on runner 0, and just the second test will run on runner 1 (skipFilename never gets set, because the ASSUMPTIONS doesn't run on that runner).

[DizzyEggg]

By atm you mean with or without this PR's changes?

[mrgriffin]

With these changes. (It would be similar without, but swapping runners 0 and 1 because return TRUE; is really return 1;)

[DizzyEggg]

I see. Now how to fix it : D

[mrgriffin]

You'd have to double-check (or I can do it later when I'm home from work), but I think just replacing return 0; with return gTestRunnerI; ought to do it 🤞

[AsparagusEduardo]

Looks like that worked (except with an extra ASSUME failed)

[AsparagusEduardo]

Do we care about fixing this extra ASSUME failed?

[AsparagusEduardo]

I applied the changes to current master and then changed:

ASSUMPTIONS
{
-   ASSUME(gBattleMoves[MOVE_BULLET_SEED].effect == EFFECT_MULTI_HIT);
+   ASSUME(gBattleMoves[MOVE_BULLET_SEED].effect == EFFECT_ABSORB);
}

[AsparagusEduardo]

Let's add the repeated assumption as a minor issue.