Skip to content

Commit

Permalink
Unit test re-organization (#1158)
Browse files Browse the repository at this point in the history
* Unit test re-organization

* awdawdadwadwdawdw

* pain, with a side of suffering

* gsfdgsffsdgdfgsdfsgfsgd

* asdfasdf

* waw

* waw2

* Let's try this.

* Collapsible success, Fucked up elsewhere

* slug

* Screenshot test priority and names

---------

Co-authored-by: Kapu1178 <[email protected]>
  • Loading branch information
francinum and Kapu1178 authored Dec 20, 2024
1 parent e47b838 commit f69c699
Show file tree
Hide file tree
Showing 149 changed files with 355 additions and 293 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci_suite.yml
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ jobs:
run: npm i
working-directory: tools/screenshot-test-comparison
- name: Run screenshot comparison
run: node tools/screenshot-test-comparison/index.js artifacts code/modules/unit_tests/screenshots artifacts/screenshot_comparisons
run: node tools/screenshot-test-comparison/index.js artifacts code/modules/unit_tests/screenshots/data artifacts/screenshot_comparisons
# workflow_run does not give you the PR it ran on,
# even through the thing literally named "matching pull requests".
# However, in GraphQL, you can check if the check suite was ran
Expand Down
2 changes: 2 additions & 0 deletions code/game/objects/items/stacks/cash.dm
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
stack_name = "wad"
multiple_gender = NEUTER

abstract_type = /obj/item/stack/spacecash

icon = 'icons/obj/economy.dmi'
icon_state = null
amount = 1
Expand Down
2 changes: 2 additions & 0 deletions code/game/objects/items/stacks/license_plates.dm
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
icon_state = "empty_plate"
novariants = FALSE
max_amount = 50
abstract_type = /obj/item/stack/license_plates


/obj/item/stack/license_plates/empty
name = "empty license plates"
Expand Down
3 changes: 3 additions & 0 deletions code/game/objects/items/stacks/sheets/mineral.dm
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ GLOBAL_LIST_INIT(sandstone_recipes, list ( \
new/datum/stack_recipe("Breakdown into sand", /obj/item/stack/ore/glass, 1, one_per_turf = FALSE, on_floor = TRUE) \
))

/obj/item/stack/sheet/mineral
abstract_type = /obj/item/stack/sheet/mineral

/obj/item/stack/sheet/mineral/sandstone
name = "sandstone bricks"
desc = "This appears to be a combination of both sand and stone."
Expand Down
2 changes: 2 additions & 0 deletions code/game/objects/items/stacks/sheets/sheets.dm
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
righthand_file = 'icons/mob/inhands/misc/sheets_righthand.dmi'
full_w_class = WEIGHT_CLASS_NORMAL

abstract_type = /obj/item/stack/sheet

force = 5
throwforce = 5
max_amount = 50
Expand Down
1 change: 1 addition & 0 deletions code/game/objects/items/stacks/tiles/tile_mineral.dm
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/obj/item/stack/tile/mineral
abstract_type = /obj/item/stack/tile/mineral
/// Determines what stack is gotten out of us when welded.
var/mineralType = null

Expand Down
1 change: 1 addition & 0 deletions code/game/objects/items/stacks/tiles/tile_types.dm
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
max_amount = 60
novariants = TRUE
material_flags = MATERIAL_EFFECTS
abstract_type = /obj/item/stack/tile
/// What type of turf does this tile produce.
var/turf_type = null
/// What dir will the turf have?
Expand Down
2 changes: 2 additions & 0 deletions code/modules/mining/ores_coins.dm
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
material_flags = MATERIAL_EFFECTS
novariants = TRUE // Ore stacks handle their icon updates themselves to keep the illusion that there's more going

abstract_type = /obj/item/stack/ore

var/list/stack_overlays
/// How many points this ore gets you from the ore redemption machine
var/points = 0
Expand Down
2 changes: 2 additions & 0 deletions code/modules/unit_tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ You can find more information about all of these from their respective doc comme

`TEST_FOCUS(test_path)` - *Only* run the test provided within the parameters. Useful for reducing noise. For example, if we only want to run our example square test, we can add `TEST_FOCUS(/datum/unit_test/square)`. Should *never* be pushed in a pull request--you will be laughed at.

`TEST_SKIP(reason)` - Mark a test as skipped, and stops the test. This is mostly used for map tests that shouldn't run on test maps, which likely have very strange setups that wouldn't be allowed in a normal, production map.

## Final Notes

- Writing tests before you attempt to fix the bug can actually speed up development a lot! It means you don't have to go in game and folllow the same exact steps manually every time. This process is known as "TDD" (test driven development). Write the test first, make sure it fails, *then* start work on the fix/feature, and you'll know you're done when your tests pass. If you do try this, do make sure to confirm in a non-testing environment just to double check.
Expand Down
31 changes: 4 additions & 27 deletions code/modules/unit_tests/__unit_tests.dm
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,18 @@
#include "_unit_test.dm"

// Category Includes
#include "atmospherics\__include.dm"
#include "combat\__include.dm"
#include "mapping_standards\__include.dm"
#include "reagents\__include.dm"
#include "screenshots\__include.dm"

// Single File Includes
#include "achievements.dm"
#include "adenosine.dm"
#include "anchored_mobs.dm"
#include "anonymous_themes.dm"
#include "area_contents.dm"
#include "atmos_moles_tests.dm"
#include "autowiki.dm"
#include "baseturfs.dm"
#include "bespoke_id.dm"
Expand All @@ -112,15 +115,6 @@
#include "circuit_component_category.dm"
#include "closets.dm"
#include "codex.dm"
#include "combat.dm"
#include "combat_blocking.dm"
#include "combat_cuffs.dm"
#include "combat_door_click.dm"
#include "combat_dropper.dm"
#include "combat_emp_flashlight.dm"
#include "combat_flash.dm"
#include "combat_pistol_whip.dm"
#include "combat_stamina.dm"
#include "component_tests.dm"
#include "confusion.dm"
#include "connect_loc.dm"
Expand All @@ -134,8 +128,6 @@
#include "egg_glands.dm"
#include "emoting.dm"
#include "food_edibility_check.dm"
#include "gas_transfer.dm"
#include "gas_validation.dm"
#include "get_turf_pixel.dm"
#include "grabbing.dm"
#include "greyscale_config.dm"
Expand Down Expand Up @@ -164,23 +156,13 @@
#include "objectives.dm"
#include "outfit_sanity.dm"
#include "paintings.dm"
#include "pills.dm"
#include "plantgrowth_tests.dm"
#include "preferences.dm"
#include "projectiles.dm"
#include "quirks.dm"
#include "rcd.dm"
#include "reagent_descriptions.dm"
#include "reagent_id_typos.dm"
#include "reagent_mod_expose.dm"
#include "reagent_mod_procs.dm"
#include "reagent_names.dm"
#include "reagent_recipe_collisions.dm"
#include "resist.dm"
#include "say.dm"
#include "screenshot_antag_icons.dm"
#include "screenshot_basic.dm"
#include "screenshot_humanoids.dm"
#include "serving_tray.dm"
#include "siunit.dm"
#include "slapcraft_sanity.dm"
Expand All @@ -189,11 +171,6 @@
#include "species_config_sanity.dm"
#include "species_unique_id.dm"
#include "species_whitelists.dm"
#include "spell_invocations.dm"
#include "spell_mindswap.dm"
#include "spell_names.dm"
#include "spell_shapeshift.dm"
#include "spell_timestop.dm"
#include "stack_singular_name.dm"
#include "stomach.dm"
#include "strippable.dm"
Expand Down
30 changes: 5 additions & 25 deletions code/modules/unit_tests/_unit_test.dm
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ GLOBAL_VAR(test_log)
GLOBAL_LIST_EMPTY(unit_test_mapping_logs)

/datum/unit_test
/// Optional 'Descriptive Name' that replaces the typepath in log messages.
var/name = null


//Bit of metadata for the future maybe
var/list/procs_tested

Expand Down Expand Up @@ -91,30 +95,6 @@ GLOBAL_LIST_EMPTY(unit_test_mapping_logs)
allocated += instance
return instance

/datum/unit_test/proc/test_screenshot(name, icon/icon)
if (!istype(icon))
TEST_FAIL("[icon] is not an icon.")
return

var/path_prefix = replacetext(replacetext("[type]", "/datum/unit_test/", ""), "/", "_")
name = replacetext(name, "/", "_")

var/filename = "code/modules/unit_tests/screenshots/[path_prefix]_[name].png"

if (fexists(filename))
var/data_filename = "data/screenshots/[path_prefix]_[name].png"
fcopy(icon, data_filename)
log_test("[path_prefix]_[name] was found, putting in data/screenshots")
else if (fexists("code"))
// We are probably running in a local build
fcopy(icon, filename)
TEST_FAIL("Screenshot for [name] did not exist. One has been created.")
else
// We are probably running in real CI, so just pretend it worked and move on
fcopy(icon, "data/screenshots_new/[path_prefix]_[name].png")

log_test("[path_prefix]_[name] was put in data/screenshots_new")

/proc/RunUnitTest(test_path, list/test_results)
var/datum/unit_test/test = new test_path

Expand All @@ -139,7 +119,7 @@ GLOBAL_LIST_EMPTY(unit_test_mapping_logs)
test_log_prefix = TEST_OUTPUT_MAGENTA("BAD STATUS [test.test_status]")

var/list/log_entry = list(
"[test_log_prefix]: [test_path] [(test.test_status != UNIT_TEST_SKIPPED ? "[duration / 10]s" : "| [test.skip_reason]")]",
"[test_log_prefix]: [test.name || test_path] [(test.test_status != UNIT_TEST_SKIPPED ? "[duration / 10]s" : "| [test.skip_reason]")]",
)
var/list/fail_reasons = test.fail_reasons
var/map_name = SSmapping.config.map_name
Expand Down
6 changes: 6 additions & 0 deletions code/modules/unit_tests/atmospherics/__include.dm
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@

#include "_atmospherics.dm"

#include "atmos_moles_tests.dm"
// #include "gas_transfer.dm" //I think this test was eaten by the above file? kapu pls
#include "gas_validation.dm"
51 changes: 51 additions & 0 deletions code/modules/unit_tests/atmospherics/_atmospherics.dm
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/datum/unit_test/atmospherics
abstract_type = /datum/unit_test/atmospherics

/datum/unit_test/atmospherics/proc/create_gas_mixes(gas_mix_data)
var/list/gas_mixes = list()
for(var/mix_name in gas_mix_data)
var/list/mix_data = gas_mix_data[mix_name]

var/datum/gas_mixture/gas_mix = new (CELL_VOLUME, mix_data["temperature"])

var/list/initial_gas = mix_data["initial_gas"]
if(initial_gas.len)
var/list/gas_args = list()
for(var/gasid in initial_gas)
gas_args += gasid
gas_args += initial_gas[gasid]
gas_mix.adjustMultipleGases(arglist(gas_args))

gas_mixes[mix_name] = gas_mix
return gas_mixes

/datum/unit_test/atmospherics/proc/gas_amount_changes(list/before_gas_mixes, list/after_gas_mixes)
var/list/result = list()
for(var/mix_name in before_gas_mixes & after_gas_mixes)
var/change = list()

var/datum/gas_mixture/before = before_gas_mixes[mix_name]
var/datum/gas_mixture/after = after_gas_mixes[mix_name]

var/list/all_gases = before.gas | after.gas
for(var/gasid in all_gases)
change[gasid] = after.getGroupGas(gasid) - before.getGroupGas(gasid)

result[mix_name] = change

return result

/datum/unit_test/atmospherics/proc/check_moles_conserved(case_name, list/before_gas_mixes, list/after_gas_mixes)
for(var/gasid in xgm_gas_data.gases)
var/before = 0
for(var/gasmix in before_gas_mixes)
var/datum/gas_mixture/G = before_gas_mixes[gasmix]
before += G.getGroupGas(gasid)

var/after = 0
for(var/gasmix in after_gas_mixes)
var/datum/gas_mixture/G = after_gas_mixes[gasmix]
after += G.getGroupGas(gasid)

if(abs(before - after) > ATMOS_PRECISION)
Fail("expected [before] moles of [gasid], found [after] moles.")
Loading

0 comments on commit f69c699

Please sign in to comment.