mirror of
https://github.com/Bubberstation/Bubberstation.git
synced 2026-01-28 01:51:46 +00:00
## About The Pull Request This PR adds a new unit test for all organs, a new unit test for lungs, and includes improvements for the existing breath and organ_set_bonus tests. Using the tests, I was able to root out bugs in the organs. This PR includes an advanced refactor of several developer-facing functions. This PR certainly represents a "quality pass" for organs which will make them easier to develop from now on. ### Synopsis of changes: 1. Fixed many fundamental bugs in organ code, especially in `Insert()`/`Remove()` and their overrides. 2. Added two new procs to `/obj/item/organ` named `on_insert` and `on_remove`, each being called after `Insert()`/`Remove()`. 3. Added `organ_effects` lazylist to `/obj/item/organ`. Converted `organ_traits` to lazylist. 2x less empty lists per organ. 4. Adding `SHOULD_CALL_PARENT(TRUE)` to `Insert()`/`Remove()` was very beneficial to stability and overall code health. 5. Created unit test `organ_sanity` for all usable organs in the game. Tests insertion and removal. 6. Created unit test `lungs_sanity` for `/obj/item/organ/internal/lungs`. 7. Improved `breath_sanity` unit tests with additional tests and conditions. 8. Improved `organ_set_bonus_sanity` unit tests with better documentation and maintainable code. --- ### Granular bug/fix list: - A lot of organs are overriding `Insert()` to apply unique side-effects, but aren't checking the return value of the parent proc which causes the activation of side-effects even if the insertion technically fails. I noticed the use-case of applying "unique side-effects" is repeated across a lot of organs in the game, and by overriding `Insert()` the potential for bugs is very high; I solved this problem with inversion-of-control by adding two new procs to `/obj/item/organ` named `on_insert` and `on_remove`, each being called after `Insert()` and `Remove()` succeed. - Many organs, such as abductor "glands", cursed heart, demon heart, alien hive-node, alien plasma-vessel, etc, were not returning their parent's `Insert()` proc return value at all, and as a result those organs `Insert()`s were always returning `null`. I have been mopping those bugs up in my last few PRs, and now the unit test reveals it all. Functions such as those in surgery expect a truthy value to be returned from `Insert()` to represent insertion success, and otherwise it force-moves the organ out of the mob. - Fixed abductor "glands" which had a hard-del bug due to their `Remove()` not calling the parent proc. - Fixed cybernetic arm implants which had a hard-del bug due to `Remove()` not resetting their `hand` variable to `null`. - Fixed lungs gas exchange implementation, which was allowing exhaled gases to feedback into the inhaled gases, which caused Humans to inhale much more gas than intended and not exhale expected gases. ### Overview of the `organ_sanity` unit test: - The new `organ_sanity` unit test gathers all "usable" organs in the game and tests to see if their `Insert()` and `Remove()` functions behave as we expect them to. - Some organs, such as the Nightmare Brain, cause the mob's species to change which subsequently swaps out all of their organs; the unit test accounts for these organs via the typecache `species_changing_organs`. - Some organs are not usable in-game and can't be unit tested, so the unit test accounts for them via the typecache `test_organ_blacklist`. ### Overview of the `lungs_sanity` unit test: - This unit test focuses on `/obj/item/organ/internal/lungs` including Plasmaman and Ashwalker lungs. The test focuses on testing the lungs' `check_breath()` proc. - The tests are composed of calling `check_breath` with different gas mixes to test breathing and suffocation. - Includes gas exchange test for inhaled/exhaled gases, such as O2 to CO2. ### Improvements to the `breath_sanity` unit tests: - Added additional tests for suffocation with empty internals, pure Nitrogen internals, and a gas-less turf. - Includes slightly more reliable tests for internals tanks. ## Why It's Good For The Game **Organs and Lungs were mostly untested. Too many refactors have been submitted without the addition of unit tests to prove the code works at all.** Time to stop. _Time to get some help_. Due to how bad the code health is in organs, any time we've tried to work with them some sort of bug caused them to blow up in our faces. I am trying to fix some of that by establishing some standard testing for organs. These tests have revealed and allowed me to fix lot of basic developer errors/oversights, as well as a few severe bugs.  ## Changelog 🆑 A.C.M.O. fix: Fixed lungs gas exchange implementation, so you always inhale and exhale the correct gases. fix: Fixed a large quantity of hard-deletes which were being caused by organs and cybernetic organs. fix: Fixed many organs which were applying side-effects regardless of whether or not the insertion failed. code: Added unit tests for Organs. code: Added unit tests for Lungs. code: Improved unit tests for breathing. code: Improved unit tests for DNA Infuser organs. /🆑
231 lines
7.5 KiB
Plaintext
231 lines
7.5 KiB
Plaintext
/*
|
|
|
|
Usage:
|
|
Override /Run() to run your test code
|
|
|
|
Call TEST_FAIL() to fail the test (You should specify a reason)
|
|
|
|
You may use /New() and /Destroy() for setup/teardown respectively
|
|
|
|
You can use the run_loc_floor_bottom_left and run_loc_floor_top_right to get turfs for testing
|
|
|
|
*/
|
|
|
|
GLOBAL_DATUM(current_test, /datum/unit_test)
|
|
GLOBAL_VAR_INIT(failed_any_test, FALSE)
|
|
GLOBAL_VAR(test_log)
|
|
/// When unit testing, all logs sent to log_mapping are stored here and retrieved in log_mapping unit test.
|
|
GLOBAL_LIST_EMPTY(unit_test_mapping_logs)
|
|
|
|
/// A list of every test that is currently focused.
|
|
/// Use the PERFORM_ALL_TESTS macro instead.
|
|
GLOBAL_VAR_INIT(focused_tests, focused_tests())
|
|
|
|
/proc/focused_tests()
|
|
var/list/focused_tests = list()
|
|
for (var/datum/unit_test/unit_test as anything in subtypesof(/datum/unit_test))
|
|
if (initial(unit_test.focus))
|
|
focused_tests += unit_test
|
|
|
|
return focused_tests.len > 0 ? focused_tests : null
|
|
|
|
/datum/unit_test
|
|
//Bit of metadata for the future maybe
|
|
var/list/procs_tested
|
|
|
|
/// The bottom left floor turf of the testing zone
|
|
var/turf/run_loc_floor_bottom_left
|
|
|
|
/// The top right floor turf of the testing zone
|
|
var/turf/run_loc_floor_top_right
|
|
///The priority of the test, the larger it is the later it fires
|
|
var/priority = TEST_DEFAULT
|
|
//internal shit
|
|
var/focus = FALSE
|
|
var/succeeded = TRUE
|
|
var/list/allocated
|
|
var/list/fail_reasons
|
|
|
|
/// Do not instantiate if type matches this
|
|
var/abstract_type = /datum/unit_test
|
|
|
|
var/static/datum/space_level/reservation
|
|
|
|
/proc/cmp_unit_test_priority(datum/unit_test/a, datum/unit_test/b)
|
|
return initial(a.priority) - initial(b.priority)
|
|
|
|
/datum/unit_test/New()
|
|
if (isnull(reservation))
|
|
var/datum/map_template/unit_tests/template = new
|
|
reservation = template.load_new_z()
|
|
|
|
allocated = new
|
|
run_loc_floor_bottom_left = get_turf(locate(/obj/effect/landmark/unit_test_bottom_left) in GLOB.landmarks_list)
|
|
run_loc_floor_top_right = get_turf(locate(/obj/effect/landmark/unit_test_top_right) in GLOB.landmarks_list)
|
|
|
|
TEST_ASSERT(isfloorturf(run_loc_floor_bottom_left), "run_loc_floor_bottom_left was not a floor ([run_loc_floor_bottom_left])")
|
|
TEST_ASSERT(isfloorturf(run_loc_floor_top_right), "run_loc_floor_top_right was not a floor ([run_loc_floor_top_right])")
|
|
|
|
/datum/unit_test/Destroy()
|
|
QDEL_LIST(allocated)
|
|
// clear the test area
|
|
for (var/turf/turf in Z_TURFS(run_loc_floor_bottom_left.z))
|
|
for (var/content in turf.contents)
|
|
if (istype(content, /obj/effect/landmark))
|
|
continue
|
|
qdel(content)
|
|
return ..()
|
|
|
|
/datum/unit_test/proc/Run()
|
|
TEST_FAIL("[type]/Run() called parent or not implemented")
|
|
|
|
/datum/unit_test/proc/Fail(reason = "No reason", file = "OUTDATED_TEST", line = 1)
|
|
succeeded = FALSE
|
|
|
|
if(!istext(reason))
|
|
reason = "FORMATTED: [reason != null ? reason : "NULL"]"
|
|
|
|
LAZYADD(fail_reasons, list(list(reason, file, line)))
|
|
|
|
/// Allocates an instance of the provided type, and places it somewhere in an available loc
|
|
/// Instances allocated through this proc will be destroyed when the test is over
|
|
/datum/unit_test/proc/allocate(type, ...)
|
|
var/list/arguments = args.Copy(2)
|
|
if(ispath(type, /atom))
|
|
if (!arguments.len)
|
|
arguments = list(run_loc_floor_bottom_left)
|
|
else if (arguments[1] == null)
|
|
arguments[1] = run_loc_floor_bottom_left
|
|
var/instance
|
|
// Byond will throw an index out of bounds if arguments is empty in that arglist call. Sigh
|
|
if(length(arguments))
|
|
instance = new type(arglist(arguments))
|
|
else
|
|
instance = new type()
|
|
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("\t[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("\t[path_prefix]_[name] was put in data/screenshots_new")
|
|
|
|
/// Helper for screenshot tests to take an image of an atom from all directions and insert it into one icon
|
|
/datum/unit_test/proc/get_flat_icon_for_all_directions(atom/thing, no_anim = TRUE)
|
|
var/icon/output = icon('icons/effects/effects.dmi', "nothing")
|
|
|
|
for (var/direction in GLOB.cardinals)
|
|
var/icon/partial = getFlatIcon(thing, defdir = direction, no_anim = no_anim)
|
|
output.Insert(partial, dir = direction)
|
|
|
|
return output
|
|
|
|
/// Logs a test message. Will use GitHub action syntax found at https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
|
|
/datum/unit_test/proc/log_for_test(text, priority, file, line)
|
|
var/map_name = SSmapping.config.map_name
|
|
|
|
// Need to escape the text to properly support newlines.
|
|
var/annotation_text = replacetext(text, "%", "%25")
|
|
annotation_text = replacetext(annotation_text, "\n", "%0A")
|
|
|
|
log_world("::[priority] file=[file],line=[line],title=[map_name]: [type]::[annotation_text]")
|
|
|
|
/proc/RunUnitTest(datum/unit_test/test_path, list/test_results)
|
|
if(ispath(test_path, /datum/unit_test/focus_only))
|
|
return
|
|
|
|
if(initial(test_path.abstract_type) == test_path)
|
|
return
|
|
|
|
var/datum/unit_test/test = new test_path
|
|
|
|
GLOB.current_test = test
|
|
var/duration = REALTIMEOFDAY
|
|
|
|
log_world("::group::[test_path]")
|
|
test.Run()
|
|
|
|
duration = REALTIMEOFDAY - duration
|
|
GLOB.current_test = null
|
|
GLOB.failed_any_test |= !test.succeeded
|
|
|
|
var/list/log_entry = list()
|
|
var/list/fail_reasons = test.fail_reasons
|
|
|
|
for(var/reasonID in 1 to LAZYLEN(fail_reasons))
|
|
var/text = fail_reasons[reasonID][1]
|
|
var/file = fail_reasons[reasonID][2]
|
|
var/line = fail_reasons[reasonID][3]
|
|
|
|
test.log_for_test(text, "error", file, line)
|
|
|
|
// Normal log message
|
|
log_entry += "\tFAILURE #[reasonID]: [text] at [file]:[line]"
|
|
|
|
var/message = log_entry.Join("\n")
|
|
log_test(message)
|
|
|
|
var/test_output_desc = "[test_path] [duration / 10]s"
|
|
if (test.succeeded)
|
|
log_world("[TEST_OUTPUT_GREEN("PASS")] [test_output_desc]")
|
|
|
|
log_world("::endgroup::")
|
|
|
|
if (!test.succeeded)
|
|
log_world("::error::[TEST_OUTPUT_RED("FAIL")] [test_output_desc]")
|
|
|
|
test_results[test_path] = list("status" = test.succeeded ? UNIT_TEST_PASSED : UNIT_TEST_FAILED, "message" = message, "name" = test_path)
|
|
|
|
qdel(test)
|
|
|
|
/proc/RunUnitTests()
|
|
CHECK_TICK
|
|
|
|
var/list/tests_to_run = subtypesof(/datum/unit_test)
|
|
var/list/focused_tests = list()
|
|
for (var/_test_to_run in tests_to_run)
|
|
var/datum/unit_test/test_to_run = _test_to_run
|
|
if (initial(test_to_run.focus))
|
|
focused_tests += test_to_run
|
|
if(length(focused_tests))
|
|
tests_to_run = focused_tests
|
|
|
|
tests_to_run = sortTim(tests_to_run, GLOBAL_PROC_REF(cmp_unit_test_priority))
|
|
|
|
var/list/test_results = list()
|
|
|
|
for(var/unit_path in tests_to_run)
|
|
CHECK_TICK //We check tick first because the unit test we run last may be so expensive that checking tick will lock up this loop forever
|
|
RunUnitTest(unit_path, test_results)
|
|
|
|
var/file_name = "data/unit_tests.json"
|
|
fdel(file_name)
|
|
file(file_name) << json_encode(test_results)
|
|
|
|
SSticker.force_ending = TRUE
|
|
//We have to call this manually because del_text can preceed us, and SSticker doesn't fire in the post game
|
|
SSticker.standard_reboot()
|
|
|
|
/datum/map_template/unit_tests
|
|
name = "Unit Tests Zone"
|
|
mappath = "_maps/templates/unit_tests.dmm"
|