Paradise
Paradise copied to clipboard
Implement map tests for catching common errors.
DO NOT MERGE, this is just a prototype mostly for feedback from @AffectedArc07 and to see how it plays with CI
What Does This PR Do
-
Adds test runner:
- to make it easier to track things across test types
- for example to ensure a fully specified log can be emitted
-
Adds map tile test type:
- when writing a test, coders implement CheckTile, which is handed a single turf
- when the test runner runs these tests, it iterates over all turfs in the specified z-level, and runs each test's CheckTile on each turf in turn.
-
Adds several sample map tile tests:
- check to see if a pipe exists on the same tile as a scrubber or vent
- check to see if a tile contains two cables, each with a center node
- check to see if a tile contains an APC bump but no center node cable
- check to see if a tile is a space turf and contains invalid objects such as airlocks
- check to see if a tile is a space turf and contains a structure but is not nearspaced
-
Adds failure log
- it was annoying during dev to try and catch errors when they were just logged to the world so here
- it's not enabled by default because it changes the expectations of the CI runners (
data/clean_run.lk
is not emitted in these circumstances). it can be enabled by passingemit_failures = TRUE
to the test runner'sFinalize()
proc.
When enabled, the failure log has a filename like data/test_run-2022-09-28T13_29_18.lk
, and looks like this:
RUN 2022-09-28T13:29:18
FAIL Unit Tests failed!
FAIL /datum/map_per_tile_test/pipe_vent_checker
127,89,2: pipe on same tile as vent or scrubber
205,106,2: pipe on same tile as vent or scrubber
FAIL /datum/map_per_tile_test/cable_node_checker
79,152,2: tile has multiple center cable nodes
80,152,2: tile has multiple center cable nodes
81,152,2: tile has multiple center cable nodes
83,152,2: tile has multiple center cable nodes
84,152,2: tile has multiple center cable nodes
85,152,2: tile has multiple center cable nodes
152,56,2: tile has multiple center cable nodes
171,50,2: tile has multiple center cable nodes
183,154,2: tile has multiple center cable nodes
PASS /datum/unit_test/aicard_icons
PASS /datum/unit_test/component_duping
PASS /datum/unit_test/config_sanity
PASS /datum/unit_test/crafting_lists
PASS /datum/unit_test/emote
FAIL /datum/unit_test/log_format
RUSTG log format is not valid 8601 format. Expected '[2022-09-28T13:29:11] Log time format test', got '[2022-09-27T23:21:23] Log time format test'
PASS /datum/unit_test/map_templates
PASS /datum/unit_test/uplink_refs
PASS /datum/unit_test/spellbook_refs
PASS /datum/unit_test/reagent_id_typos
PASS /datum/unit_test/rustg_version
PASS /datum/unit_test/spawn_humans
PASS /datum/unit_test/spell_targeting
FAIL /datum/unit_test/sql_version
SQL errors occured on startup. Please fix them.
PASS /datum/unit_test/subsystem_init
PASS /datum/unit_test/subsystem_metric_sanity
PASS /datum/unit_test/timer_sanity
Why It's Good For The Game
Automating checks for common map errors reduces cognitive overhead and maintenance burden for mappers and reviewers, and the payoff will extend forever into the future.
Testing
- Ran tests locally against all maps, and made fixes to all maps (#19208, #19211, #19213)
- Ran tests on CI
Changelog
:cl: add: Start building test suite specifically for map linting. /:cl:
this is what they call mega poggers
yes please
PTAL. Also, currently it appears the "CI / Unit Tests + SQL Validation" will automatically cancel the unit tests for all maps when one returns a failure, instead of letting them all run to completion? I'm not sure if we want to keep that or not.
@AffectedArc07 friendly ping about the test failure strategy for CI; it makes the most sense to set https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstrategyfail-fast to false for these tests as now they can return different results per map, and we don't want to kill the whole matrix when one fails because that would obscure the failures on other maps. If you'd like I can make that change here or after the fact.
Yeah make it not kill all of them. Thats been an issue I have had for a while but never got round to fixing.
Yeah make it not kill all of them. Thats been an issue I have had for a while but never got round to fixing.
Should be good now.
We should also test for multiple turfs on a given tile. This is technically legal as one turf will get added to the vis_contents of the other. But it leads to issues and we generally should forbid it.
We should also test for multiple turfs on a given tile. This is technically legal as one turf will get added to the vis_contents of the other. But it leads to issues and we generally should forbid it.
@S34NW Since you have really in-depth mapping knowledge could you toss a to-do list in maybe a GitHub discussion of tests that should be added after this? Adding more tests now will mean the chance that existing maps will fail, which will require me to fix those in separate PRs before pulling this PR back to master in order for it to pass CI.