Event interpreter unit testing

[Ghabry]

Many times we observed now that old interpreter issues that got fixed are broken due to new bugfixes.

Because the interpreter is the most fragile part of our Player I would suggest new "unit tests" which consist of a minimal database, and a single (or 2 for teleporting) map file containing an event.

Things to do:

• Write a DynRPG plugin that traces event execution, this is the reference file.
• Make changes to Player that allow Event unit testing
• Compare both files -> Test passed when equal

Tests could be executed by using --map-id and --new-game

[Ghabry]

Made some progress

DynRPG code repo: https://github.com/Ghabry/dynrpg-eventtracer

Changes made to Player: https://github.com/Ghabry/easyrpg-player/tree/event_unittest

[Ghabry]

Almost finished, I need some help from automake experts ;) @fdelapena @carstene1ns

The TESTS need a different working directory:
Must be "tests/" otherwise the event tracer will assert (because it can't find the trace log file)

How to supply command line arguments to tests? At least --project-path will be required.

https://github.com/Ghabry/easyrpg-player/blob/event_unittest/Makefile.am#L328

[Ghabry]

Move Route logging is now implemented in DynRPG plugin (via custom ASM code).
Only needs the Player part now (much easier ;))

[Ghabry]

What do you think? Should the traces (the log files) be stored in the event-test game or in the tests folder of player?

[fdelapena]

How worth is separating event trace tests from Player tests?
IMO if there are too many event trace tests it might be worth to have them separated and keep Player tests more Player specific, however having 2 different repository tests affecting Player behavior might be confusing, if we want to run tests when doing "make check" or "make distcheck" from the tarball source itself then I think it should be in the same repository for easier generation.

About running tests from the tests/ folder: Creating a Makefile.am in tests/ and adding tests/Makefile to AC_CONFIG_FILES() in configure.ac (it might need to re-run autoreconf to regenerate configure.in) and move sometest_SOURCES = tests/sometestsource.cpp to the new tests/Makefile.am without the tests/ part: sometest_SOURCES = sometestsource.cpp. About passing parameters, I've linked some days ago a way described in stackoverflow using shell scripts instead of calling the binary executable directly.

[Ghabry]

Well, many tests will fail (e.g. FileFinder) because they require a valid project. So it doesn't really matter if the trace files are in the game project or in player, both must be kept on sync.
Another, simpler solution is placing everything in a test subfolder... but don't really want to clutter the Player repo with that.

[fdelapena]

Ok, then I guess the best option is have it in the separate repository and keep its own Automake build files to keep Player's "make check" work properly without those game-dependent tests.

[Ghabry]

Just noticed, that I don't need command line at all... The project_path can be overwritten via Env var RPG_GAME_PATH :D, upps

[Ghabry]

Well there is still this issue around. For really serious serious testing there are different things that could be checked. As we are much more advanced now we can go deep into testing:

1. Frame-by-Frame compare: Take screenshots each frame (synched by frame counter chunk) and do a pixel-by-pixel compare
2. Event/Movement tracing: Compare all executed events/movements per frame
3. Savegame compare: Compare saves for equality

Required RPG_RT hooking. Outside of DynRPG - Should be universal.

• Replace all text with empty or X because otherwise image compare will fail. (getting the font right is not worth it)
• 32 bit framebuffer patch -> Extract image each frame
• Save game each frame
• Reading input from a input file recorded by the Player
• Random number generator -> Maybe use a fake generator that always returns the same number, otherwise the amount of calls must be equal between Player and RT which is hard/impossible to get right

[fmatthew5876]

What might be easier to maintain is a comprehensive set of unit tests. By black box testing, using RE tools, and the github issue list, we can understand the RPG_RT behavior and come up with a set of tests. Once we have the correct behavior encoded in unit tests, we don't need to maintain playback recordings to compare with RPG_RT.

The biggest blocker to having a test suite is the use of global variables. This makes it very difficult to setup a mock environment to do isolated tests of components. Our code is in much better shape now than in previous generations, but we still have way too many tangled dependencies. Refactoring our code for testability will also force us to modularize our code.

We should also do fuzz testing against our interpreter. We can fuzz the input parameter array. This will harden our engine against crashing on bad interpreter code and expose edge case behaviors.

So to summarize:

1. Come up with and write a comprehensive set of unit tests for each component (map, character, interpreter, each event command etc..)
2. Everytime a new bug is fixed, the PR author must add passing unit test cases that reproduce the bug.
3. Fuzz testing for interfaces which take numbers such as interpreter commands, flash screen, etc..