Testing Forge

[ptx]

As I said last week, I've been looking into writing integration tests for Forge, with the end goal being to have a bunch of tests that set up an initial game situation, then have fake players take some actions, then check the game state a bit to verify that the rules/cards were correctly implemented.
Since I'm new, I obviously spent most of my time just getting acquainted with how Forge works.

The main obstacle I see at this point is related to what I had initially feared (UI stuff embedded in game logic etc), but actually in the opposite direction (ie, game logic embedded in what is essentially UI stuff)!
My fake players would be dummy implementations of the PlayerController abstract class, parallel to PlayerControllerHuman and PlayerControllerAI. Ideally those classes would have nothing to do with game logic, they would just be responsible for making choices (through the GUI for the Human version, through AI logic for the AI version, and through test-specific code for the test version). In reality however, there is quite a lot of game logic embedded in these classes.
This is obviously a problem for testing, as you're forced to replicate lots of game logic in your test in order to get the correct behaviour. However, it's also a danger for real code, because the logic is already duplicated between the two existing controllers, and if updates aren't done correctly, incorrect behaviour can occur, as the real game logic does not check if what happens in the controllers is legal.

Some examples :
In the takePriority function, you can theoretically drop lands into play whenever you want, cast spells you don't even have, without casting mana cost, or with totally invalid targets, ...
In the declareAttackers function, you can theoretically attack with Walls, creatures with summoning sickness, ...

Solving this problem in the "real code" is a pretty huge refactoring. It's one that I'm pretty sure we're going to want to do eventually, but it's gonna take a while, and it should probably be discussed a bit beforehand, to make sure that everyone will be happy with the final result
Working around the problem by making the test implementation smart is probably considerably less work, especially if you consider that I'm just getting started with the tests, so I can probably get by with a limited set of functionality for now. Though obviously the work is "wasted" in the long run, as the refactoring will eventually make it obsolete.

Thoughts? Opinions?

[ptx]

I just made a first commit with some initial tests (and test helper code). Took a bit longer than expected due to RL issues, but better late then never...
Everything works fine locally, but if the build system chokes on anything I would obviously appreciate swift feedback

There's about a dozen tests, based on section 103 and 104 of the comprehensive rules, mainly basic stuff, but probably already nice to have around as sanity checks.
One test fails : ComprehensiveRulesSection104.test_104_3f_if_a_player_would_win_and_lose_simultaneously_he_loses , my impression is that this is indeed a bug in Forge. However, this bug is hardly ever going to be relevant, so for now I just disabled the test.

The helper code contains some pretty ugly (and sometimes outright wrong) stuff, but I did not yet want to modify any "real" code, so I just pushed all the needed uglyness into this helper code for now. Obviously I hope that eventually we can refactor the real code, so that some of the junk in the test helper code can be cleaned up.
In addition to the stuff mentioned in the previous post (which I worked around by just hardcoding just what I needed, even though this basically violates game rules etc), I also noticed that loading a card automatically loads it's image (for now I just set up an ugly mocking construction to avoid this).