Flaky TryToCompleteTest.testRewardChance0NoItems, and level rewards bypass the reward-chance gate

[tastybento]

Two related things, found while merging #430. Neither is caused by that PR — it only touches build config.

1. TryToCompleteTest.testRewardChance0NoItems is intermittently flaky

Failed once in CI, then passed on a rerun of the identical commit:

• Run 1 (commit 64a98f4) — Tests run: 522, Failures: 1 — TryToCompleteTest.testRewardChance0NoItems:967
• Rerun, same commit — Tests run: 522, Failures: 0

Locally it passes 5/5 in isolation (-Dtest=TryToCompleteTest) and in two full-suite runs on both JDK 21 and JDK 25.

The failing assertion is the first verify in the test:

setRewardChanceField(challenge, 0);
...
assertTrue(TryToComplete.complete(addon, user, challenge, world, topLabel, permissionPrefix));

// Should not add reward items
verify(inv, never()).addItem(any());   // <-- line 967

So inv.addItem(...) was called on a challenge whose reward chance is 0.

Ruled out so far:

• Randomness. shouldRewardItems() short-circuits on chance <= 0 and returns false before ever touching random.nextInt(100). The 0 case is deterministic.
• Stub leaking between tests. cm.tryCompleteLevel(...) is stubbed to return a level at lines 869 and 1092, which looked like the obvious culprit — but AbstractChallengesTest calls MockitoAnnotations.openMocks(this) in @BeforeEach and closes it in @AfterEach, so every @Mock is rebuilt per test.
• Parallel execution / test ordering. Surefire has no parallel, forkCount, threadCount or runOrder configuration.

That leaves state that outlives a single test class — MockBukkit or a BentoBox singleton — or something genuinely environment-dependent on the runner. I have not pinned it down.

Worth noting the same run also had a forked-VM crash locally on one occasion that did not recur, which may or may not be the same underlying instability.

2. Level-completion rewards bypass the reward-chance gate

Independent of the flake, and verifiable by reading:

Every reward path in TryToComplete is gated by shouldRewardItems() — first-completion rewards roll once at line 263, repeat rewards roll per iteration at line 350. But the level rewards are not:

https://github.com/BentoBoxWorld/Challenges/blob/develop/src/main/java/world/bentobox/challenges/tasks/TryToComplete.java#L420-L431

ChallengeLevel level = this.manager.tryCompleteLevel(this.user, this.world, this.challenge);

if (level != null)
{
    // Item rewards
    for (ItemStack reward : level.getRewardItems())
    {
        this.user.getInventory().addItem(reward.clone())...

No shouldRewardItems() check. So completing the final challenge in a level always pays out the level reward in full, even at rewardChance 0.

This is also the only ungated addItem call reachable once requirements are met, which makes it the natural suspect for #1 — but with mocks rebuilt per test I cannot currently explain how tryCompleteLevel would return non-null there.

The question is whether this is intended. A reasonable argument either way: reward chance is documented as a per-challenge gamble, and a level reward is arguably a separate milestone payout that shouldn't be subject to it. If that is the intent, it is worth a comment in the code and a line in the docs, because the current behaviour is silent. If it isn't, the gate belongs there.

Suggested next steps

• Reproduce Multigame Update #1 by looping the full suite in CI (mvn test xN) rather than the single class, since it does not reproduce in isolation.
• Decide the intent on duplication in locale file? #2 and either add the gate or document the exemption.