Testing requirements before merging to master

[TiMMyyMMiT]

Because of some GitHub conversations (#28 (comment) and #41 (comment))
I suspect that we are now more likely to encounter issues where Disasm changes can break the tools (as well as general testing to reduce regression issues in tools). With a goal of avoiding these kinds of issues, I want to discuss possible testing requirements before pushing to master.

Testing requirements

Having standard tests in place will make it easier for us to catch breaking changes and regression issues.

However, we have a unique issue that automated tests will be difficult, if not impossible, because we cannot include the rom files inside the repo. There are some alternatives though:

• PR Templates are a simple way to include a checklist of manual tests that should be performed by the PR reviewer (and, honestly, by whomever made the changes as well)
• Batch script to automatically run some/all of these tests (the .jar tools should be able to be run through commands)
• The Batch script option could be combined with a git pre-push hook. These can be great but require per-user setup so are prone to issues
• Automation may be an option in the future (e.g. pre-push hook to run testing batch scripts)

Implementation plan

Of course, it would take time to implement any kind of [semi]automation so my proposal would be to start simple, with a goal of reaching automation:
Step 1: GitHub PR Template + manual testing
Step 2: Manually run batch scripts to test specific functionality (expanding as needed)
Step 3: Some form of local-side automation (pre-push hooks or just a one-click-test-all batch script)

Standard Tests (first proposal)

In terms of the standard tests, I am thinking of the following:

1. Run build.bat and confirm that it produces a bit-perfect replica of original rom
2. Run buildstandard.bat and confirm that build succeeds
3. Open MapEditor, load map63 and export then import again (confirm that data exports without issue and import as-it-was)
4. Open BattleSpriteEditor and load a character then export as pngs then import pngs back in (confirm that image import/export works without issue)
5. Open BattleEditor, load a battle (battle00) (This one uses SF2Enums heavily so this is a good test that it can read all of this)
   (Is this too much to test? Are we missing any critical tests?)
   I would want to include the MapCreator but it has too many steps to be usable (since you need to export from MapEditor first)

Questions:

So, ultimately, my questions are:

1. Do we see it as necessary to put some work in to prevent the master branch from errors or broken tools?
   • The alternative to all of this is to set semi-regular release versions of SF2DISASM repo (instead of expecting users to clone master)
2. Are we happy with a plan towards automation? (PR templates -> manual batch scripts -> possible local-side automation)
3. Anything significant missing from the standard tests?

[xenometal]

I do agree that we've seemingly reached a point where some form of communication should be established to better coordinate our efforts between the disassembly and the Java tools. To that end, I followed your recommendation and created a PR template that includes a checklist of standard tests to be performed. (See PR #49.)

Some form of automation being implemented does sound appealing, especially if the number of tests is to grow beyond those already proposed. For that purpose, manual batch scripts alone might suffice.