Should we introduce a way for mapmakers to produce engine tests?

[asvitkine]

@DanVanAtta @RoiEXLab and others, I'd like to get your thoughts on this.

We know that TripleA lacks a lot of test coverage. So it's easy to introduce bugs and hard to find them without manual testing.

At the same time, mapmakers and rule experts sometimes help test new engine versions and report bugs. However, this is pretty manual and then the issue needs to be reproduced and confirmed / debugged by a developer. Sometimes there is user error involved where someone may claim there's a bug somewhere, but they made a mistake in testing, etc.

Then, when it comes time to fixing something, we don't always add a test case to cover the behavior - it's often a lot of work to do so.

So, my idea is this - what if we introduce the ability for mapmakers who don't know Java programming to produce test cases that the engine could run? They can be specified in XML or some other declarative language, where programming is not required.

For example, the test can specify a specific map to use and to set up the board state (e.g. set phase, add units, do moves). Then it can provide some expectations - for example, a simple test could be something like:

<game-test>
  <map url="https://github.com/triplea-maps/world_war_ii_global/archive/master.zip"/>
  <step name="americanCombatMove"/>
  <setupTerritory name="Northern Italy" removeExistingUnits="true">
    <unit name="factory_major" owner="Italians"/>
  </setupTerritory>
  <setupTerritory name="95 Sea Zone" removeExistingUnits="true">
    <unit name="transport" owner="Americans">
      <unit name="infantry" owner="Americans"/>
    </unit>
  </setupTerritory>
  <performMoves>
    <move from="95 Sea Zone" to="Northern Italy">
      <unit name="infantry" owner="Americans"/>
    </move>
  </performMoves>
  <advanceToStep name="americanNonCombatMove"/>
  <verifyTerritory name="Northern Italy">
      <owner player="Americans"/>
      <unitPresence name="infantry" owner="Americans"/>
      <unitPresence name="factory_minor" owner="Americans"/>
  </verify>
</game-test>

Something like the above can be authored by mapmakers when reporting bugs. When we fix the bugs, the test can be added to the repository expanding test coverage. WDYT?

[RoiEXLab]

I think the idea goes in the right direction, however I think your proposal does try to achieve too many things at once.

Do we need more regression testing? Yes, definitely!

Do I think those kind of regression tests should count towards coverage? Probably not, I think this would create a false sense of correctness for helper functions that work in the common cases, but actually miss out to check edge cases that might be never touched in a test scenario.

How easy should it be to setup those tests? This is where I think you're raising a pretty good point. I personally have no Idea on how to properly create a test scenario in code. I would probably be able to copy existing test cases and fiddle around with the values a bot to achieve the desired result, but from scratch I'd be pretty clueless. So having some sort of structured setup for test cases could come in pretty handy. But as mentioned earlier this is where we need to decide which direction we want to chose. Do we want to test certain functions pretty close to unit tests where coverage is relevant, or do we want to provide map creators and/or power-users with the tools to simulate small to medium-sized scenarios? Those are 2 different things, both with their own purpose.

Also I think yaml suits this task more than xml.

[asvitkine]

Do I think those kind of regression tests should count towards coverage? Probably not, I think this would create a false sense of correctness for helper functions that work in the common cases, but actually miss out to check edge cases that might be never touched in a test scenario.

I was thinking these would be "smoke tests", which currently don't count for coverage.
TBH, I think it's still useful to have separate code coverage stats for smoke tests - e.g. if we had a collection of tests like these, we can still look at which code they're covering and which gameplay areas they're not covering so we can expand that collection of tests.

But I agree that it's not the same as unit test coverage, so having two separate coverage measures would make sense to me. Unit test coverage is definitely useful, but also doesn't catch some of the issues around how different features interact together - case in point #9962 which had good unit tests, but no integration tests and didn't take into account how it would interact with other engine features, causing problems.

How easy should it be to setup those tests? This is where I think you're raising a pretty good point. I personally have no Idea on how to properly create a test scenario in code. I would probably be able to copy existing test cases and fiddle around with the values a bot to achieve the desired result, but from scratch I'd be pretty clueless. So having some sort of structured setup for test cases could come in pretty handy.

Indeed and I think the biggest benefit is we could enlist the mapmaking community to help - as they could easily report gameplay bugs by attaching new test cases - which means the overall effort of improving the test suite will have more contributors.

But as mentioned earlier this is where we need to decide which direction we want to chose. Do we want to test certain functions pretty close to unit tests where coverage is relevant, or do we want to provide map creators and/or power-users with the tools to simulate small to medium-sized scenarios? Those are 2 different things, both with their own purpose.

As I mentioned above, I do think coverage of these test suite would be useful information, to know which engine features are missing test cases. But I think it should be separate from unit test coverage.

I do think allowing non-developers to simulate scenarios should be something this should allow.

Also I think yaml suits this task more than xml.

Any particular reason? In yaml, everything is on its own line, right? So in my example, for adding a unit, we'd need at least 3 lines, wouldn't we? That would make the test case less concise and harder to read imho, so I think XML is better (but perhaps something else could perhaps be even more so - although one advantage of XML is that it's also used for maps, so mapmakers wouldn't need to learn a different format if they're already familiar with XML).

[RoiEXLab]

Smoke testing sounds good and the separate coverage sounds like an interesting idea (I don't think our tools support this out of the box though).

In yaml, everything is on its own line, right?

Not necessarily. As of yaml 1.2 json is a subset of yaml which means that you can use json notation within yaml to shorten lists or key-value pairs. The main benefit in my opinion is the reduced boilerplate you can avoid in yaml. Closing tags are not required for example and if you have a list of the same tags (units for example) there's no need to repeat the type "unit" for every individual entry and you can just provide the actual content that differs.

In the end it's just personal preference though. I personally think having new lines for properties is not even a bad thing, modern editors allow you to collapse item trees if they are in the way

[DanVanAtta]

Sorry to not have enough time right now for a very detailed response. To add some fodder to this conversation:

• do we have any specific map makers who have volunteered to help with this kind of thing? Can we actually expect map makers to make these tests (in practice)? I feel like the previous experiences trying to outsource some aspects of TripleA could pose challenges (it's been difficult to get map makers to submit any kind of PRs, let alone validating that a test works).

• I do like how these are cucumber-like tests for TripleA. Would cucumber be a better fit to drive these tests?

• How can we make the tests very durable? Notably such that they do not need to change greatly if/when we evolve the game engine to work in 3.0.

• Are we potentially building the wrong tool if we can test these scenarios at different levels? For example, we could set up a movement scenario with a game data and then use the movement validator module to verify that a given move is valid. Perhaps we should be focusing on making the movement validator module more modular, so we could test these types of scenarios morewith ' easily without reconstructing an entire game data object.

• test speed, would these tests potentially be very slow for the amount of test coverage that we gain? If we have a lot of these tests, I suspect they could become quite slow overall.

Overall, I think TripleA needs to be re-modularized and that would allow for a lot of new/better tests than what we can do now as compared to the current spagettic & BBOM structure. 3.0 would operate largely 'game state change objects', those would be serialized as plain text and could be crafted for these kind of test cases. I somewhat think then that if we accomplish that, we could pretty readily say "given this map, and this change state change, validate that the change works", or "validate that the territory owner is now X".

Another concern is that we might invest and/or lock ourselves into a pattern that we later need to throw away or would want to drastically change - where we could potentially invest in 3.0 directly and then have this kind of thing be easier and much more natural compared to what we would need to build today.

[asvitkine]

Sorry to not have enough time right now for a very detailed response. To add some fodder to this conversation:

• do we have any specific map makers who have volunteered to help with this kind of thing? Can we actually expect map makers to make these tests (in practice)? I feel like the previous experiences trying to outsource some aspects of TripleA could pose challenges (it's been difficult to get map makers to submit any kind of PRs, let alone validating that a test works).

I think once we have a framework like this in place, we can just ask people reporting gameplay bugs to formulate them as test cases. I don't think we necessarily even need to get them to use Git to add them - we could have a tool in the engine itself to test a given XML snippet - and they can just copy-paste them in bug reports.

Worst case scenario, developers can create them if a bug reporter isn't willing - anyway, it's a useful way to repro the issue in a controlled scenario that doesn't have as much room for user errors.

• I do like how these are cucumber-like tests for TripleA. Would cucumber be a better fit to drive these tests?

Sorry, I'm not familiar with cucumber. Possibly? But if not, can just be done by smoke tests.

• How can we make the tests very durable? Notably such that they do not need to change greatly if/when we evolve the game engine to work in 3.0.

If you look at my XML snippet, it actually doesn't really depend on the actual existing XML syntax for maps much. It's orthogonal and just logically describes the board and the expectations. So it could theoretically be applied to a completely different game engine that can implement a given ruleset.

The dependencies it currently has:

• It does specify the path to the .zip file for the map (so it does exercise the logic to load the map, which obviously has the current syntax, but it's abstracted away and if we add support for 3.0 maps, we can change it to point to a 3.0 map).
• It has specific string names for units and territories, so this is map-specific (e.g. a different engine / map could call it "Minor Factory" instead "factory_minor" - but that's pretty benign and wouldn't change with 3.0)
• It does name the specific step (i.e. "americanNonCombatMove"), so this could change if we move away from the current step naming system - but that would depend on the new map XML anyway and would be a benign update.

So overall, I'm not worried about this being tied to 1.9 XML format - because it's generally not tied. Obviously, as we expand the capabilities of this framework, we have to be careful to not introduce such dependencies. I think a good rule of thumb is to express the user visible characteristics (e.g. which units are on the board, how many dice are rolled) and not behind-the-scenes implementation-specific things (e.g. the fact that there are specific attachments and their properties).

• Are we potentially building the wrong tool if we can test these scenarios at different levels? For example, we could set up a movement scenario with a game data and then use the movement validator module to verify that a given move is valid. Perhaps we should be focusing on making the movement validator module more modular, so we could test these types of scenarios morewith ' easily without reconstructing an entire game data object.

I think some of these issues are beyond what can be tested with MoveValidator, for example the case above was about an interaction of different changes produced by the engine and what they do. So even testing a "sequence of changes" is not sufficient, since you want to test the changes that are generated too (and in which order etc).

• test speed, would these tests potentially be very slow for the amount of test coverage that we gain? If we have a lot of these tests, I suspect they could become quite slow overall.

I think the actual engine execution would actually be pretty fast, the parts that could be slow is loading game XMLs and possibly downloading maps. These could be mitigated by minimizing the work done (for example, only loading game XML once and cloning it for all tests that use the same one).

Overall, I think TripleA needs to be re-modularized and that would allow for a lot of new/better tests than what we can do now as compared to the current spagettic & BBOM structure. 3.0 would operate largely 'game state change objects', those would be serialized as plain text and could be crafted for these kind of test cases. I somewhat think then that if we accomplish that, we could pretty readily say "given this map, and this change state change, validate that the change works", or "validate that the territory owner is now X".

Another concern is that we might invest and/or lock ourselves into a pattern that we later need to throw away or would want to drastically change - where we could potentially invest in 3.0 directly and then have this kind of thing be easier and much more natural compared to what we would need to build today.

I actually think improving tests first is pretty critical for making sure any kind of large re-architecture project is successful. I think if you do it in the opposite order, you're risking breaking lots of things and not knowing about it until much later where it takes much more work to figure out what happened and how to fix it. So I'd advocate something like this being put on the critical path before any large refactoring / engine changes.

[TheDog-GH]

I will volunteer to help with this, for a short time.

We could test each of the properties in the xml and that should be portable to v3, yes?