From cfc75f5a848f1194e67ffe606ea42f4c9eacbb33 Mon Sep 17 00:00:00 2001 From: LemonInTheDark <58055496+LemonInTheDark@users.noreply.github.com> Date: Fri, 14 May 2021 16:32:13 -0700 Subject: [PATCH] Fixes some more holes in the ref tracker (#58972) * Fixes some more holes in the ref tracker The reference tracker was failing to check null keyed assoc list entries, along with being unable to check both lists in a list(list() = list()) pair This resolves that, and adds some new logic to the unit test to check for this sort of thing * Seperates the ref tracking unit test into 6 subtasks as requested --- .../view_variables/reference_tracking.dm | 17 ++++--- .../unit_tests/find_reference_sanity.dm | 49 +++++++++++++------ 2 files changed, 43 insertions(+), 23 deletions(-) diff --git a/code/modules/admin/view_variables/reference_tracking.dm b/code/modules/admin/view_variables/reference_tracking.dm index 93cdb4991ab..c7129a1ebae 100644 --- a/code/modules/admin/view_variables/reference_tracking.dm +++ b/code/modules/admin/view_variables/reference_tracking.dm @@ -118,19 +118,20 @@ testing("Found [type] \ref[src] in list [container_name].") //Check assoc entrys - else if(element_in_list && !isnum(element_in_list) && normal && potential_container[element_in_list] == src) + else if(!isnum(element_in_list) && normal && potential_container[element_in_list] == src) #ifdef REFERENCE_TRACKING_DEBUG found_refs[potential_container] = TRUE #endif testing("Found [type] \ref[src] in list [container_name]\[[element_in_list]\]") - //Check normal sublists - else if(islist(element_in_list)) - DoSearchVar(element_in_list, "[container_name] -> [element_in_list] (list)", recursive_limit - 1, search_time) - - //Check assoc sublists - else if(element_in_list && !isnum(element_in_list) && normal && islist(potential_container[element_in_list])) - DoSearchVar(potential_container[element_in_list], "[container_name]\[[element_in_list]\] -> [potential_container[element_in_list]] (list)", recursive_limit - 1, search_time) + //We need to run both of these checks, since our object could be hiding in either of them + else + //Check normal sublists + if(islist(element_in_list)) + DoSearchVar(element_in_list, "[container_name] -> [element_in_list] (list)", recursive_limit - 1, search_time) + //Check assoc sublists + if(!isnum(element_in_list) && normal && islist(potential_container[element_in_list])) + DoSearchVar(potential_container[element_in_list], "[container_name]\[[element_in_list]\] -> [potential_container[element_in_list]] (list)", recursive_limit - 1, search_time) #ifndef FIND_REF_NO_CHECK_TICK CHECK_TICK diff --git a/code/modules/unit_tests/find_reference_sanity.dm b/code/modules/unit_tests/find_reference_sanity.dm index e958c1e2ddf..53cccea9e7b 100644 --- a/code/modules/unit_tests/find_reference_sanity.dm +++ b/code/modules/unit_tests/find_reference_sanity.dm @@ -16,8 +16,10 @@ //Sanity check victim.DoSearchVar(testbed, "Sanity Check", search_time = 1) //We increment search time to get around an optimization TEST_ASSERT(!victim.found_refs.len, "The ref-tracking tool found a ref where none existed") - victim.found_refs.Cut() +/datum/unit_test/find_reference_baseline/Run() + var/atom/movable/ref_test/victim = allocate(/atom/movable/ref_test) + var/atom/movable/ref_holder/testbed = allocate(/atom/movable/ref_holder) //Set up for the first round of tests testbed.test = victim testbed.test_list += victim @@ -28,10 +30,10 @@ TEST_ASSERT(victim.found_refs["test"], "The ref-tracking tool failed to find a regular value") TEST_ASSERT(victim.found_refs[testbed.test_list], "The ref-tracking tool failed to find a list entry") TEST_ASSERT(victim.found_refs[testbed.test_assoc_list], "The ref-tracking tool failed to find an assoc list value") - victim.found_refs.Cut() - testbed.test = null - testbed.test_list.Cut() - testbed.test_assoc_list.Cut() + +/datum/unit_test/find_reference_exotic/Run() + var/atom/movable/ref_test/victim = allocate(/atom/movable/ref_test) + var/atom/movable/ref_holder/testbed = allocate(/atom/movable/ref_holder) //Second round, bit harder this time testbed.overlays += victim @@ -44,10 +46,10 @@ TEST_ASSERT(!victim.found_refs[testbed.overlays], "The ref-tracking tool found an overlays entry? That shouldn't be possible") TEST_ASSERT(victim.found_refs[testbed.vis_contents], "The ref-tracking tool failed to find a vis_contents entry") TEST_ASSERT(victim.found_refs[testbed.test_assoc_list], "The ref-tracking tool failed to find an assoc list key") - victim.found_refs.Cut() - testbed.overlays.Cut() - testbed.vis_contents.Cut() - testbed.test_assoc_list.Cut() + +/datum/unit_test/find_reference_esoteric/Run() + var/atom/movable/ref_test/victim = allocate(/atom/movable/ref_test) + var/atom/movable/ref_holder/testbed = allocate(/atom/movable/ref_holder) //Let's get a bit esoteric victim.self_ref = victim @@ -61,9 +63,26 @@ TEST_ASSERT(victim.found_refs["self_ref"], "The ref-tracking tool failed to find a self reference") TEST_ASSERT(victim.found_refs[to_find], "The ref-tracking tool failed to find a nested list entry") TEST_ASSERT(victim.found_refs[to_find_assoc], "The ref-tracking tool failed to find a nested assoc list entry") - victim.found_refs.Cut() - victim.self_ref = null - to_find.Cut() - testbed.test_list.Cut() - to_find_assoc.Cut() - testbed.test_assoc_list.Cut() + +/datum/unit_test/find_reference_null_key_entry/Run() + var/atom/movable/ref_test/victim = allocate(/atom/movable/ref_test) + var/atom/movable/ref_holder/testbed = allocate(/atom/movable/ref_holder) + + //Calm before the storm + testbed.test_assoc_list = list(null = victim) + + victim.DoSearchVar(testbed, "Fourth Run", search_time = 5) + TEST_ASSERT(testbed.test_assoc_list, "The ref-tracking tool failed to find a null key'd assoc list entry") + +/datum/unit_test/find_reference_assoc_investigation/Run() + var/atom/movable/ref_test/victim = allocate(/atom/movable/ref_test) + var/atom/movable/ref_holder/testbed = allocate(/atom/movable/ref_holder) + //Let's do some more complex assoc list investigation + var/list/to_find_in_key = list(victim) + testbed.test_assoc_list[to_find_in_key] = list("memes") + var/list/to_find_null_assoc_nested = list(victim) + testbed.test_assoc_list[null] = to_find_null_assoc_nested + + victim.DoSearchVar(testbed, "Fifth Run", search_time = 6) + TEST_ASSERT(victim.found_refs[to_find_in_key], "The ref-tracking tool failed to find a nested assoc list key") + TEST_ASSERT(victim.found_refs[to_find_null_assoc_nested], "The ref-tracking tool failed to find a null key'd nested assoc list entry")