r25794 - Add unit tests for SnapperCommand (#138)

fewyn

Administrator
Staff member
3 failed tests which is working as intended and not archiving the build of Jenkins.

Bash:
> Task :test

CustomScriptTest > testScript(String) > net.sourceforge.kolmafia.CustomScriptTest.testScript(String)[11] FAILED
    org.opentest4j.AssertionFailedError at CustomScriptTest.java:59

MaximizerTest > maximizeShouldNotRemoveEquipmentThatCanNoLongerBeEquipped() FAILED
    org.opentest4j.AssertionFailedError at MaximizerTest.java:102

MaximizerTest > maximizeGiveBestScoreWithEffectsAtNoncombatLimit() FAILED
    org.opentest4j.AssertionFailedError at MaximizerTest.java:74

529 tests completed, 3 failed, 7 skipped

> Task :test FAILED
 

heeheehee

Developer
Staff member
CustomScriptTest: "testUser wins the fight!" -- sounds like testUser is leaking into this test case.

re: MaximizerTest: something is apparently introducing an extra source of noncombat, although I haven't figured out what.
 

MCroft

Developer
Staff member
Yeah, we’ve been sniping leaky resets all day. This is difficult for me to test, because my Mac keeps passing the damn tests, but we have what we hope are some useful test scaffolding code (a @BeforeAll and @AfterAll fn) to keep the test threads clean despite the changes we’re making…

PR 136 adds a guardrail to customScriptTest. We can do the same to maximizerTest
 

heeheehee

Developer
Staff member
My biased opinion is that adding defensive resets to unrelated tests is the wrong place to make these changes. Either fix the problem at the source, or fix it systematically in the test runner framework.
 

MCroft

Developer
Staff member
My biased opinion is that adding defensive resets to unrelated tests is the wrong place to make these changes. Either fix the problem at the source, or fix it systematically in the test runner framework.
Ok, so in test, we want to start from a clean, predictable state. especially with the customScriptTest, which is an all-or-nothing text compare. Tests should either be able to count on a state at start or set one.

I do like the leave cleanly function (especially if it works), but do we have a parallel test issue?
 

heeheehee

Developer
Staff member
Tests don't run in parallel by default.

There are so many non-synchronized accesses that doing so is most likely fraught with peril.
 
Top