From 207ec20f83637d8d8c2f5a1cbf8ef600fd0c2cf4 Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Tue, 1 Sep 2026 18:56:52 -0300 Subject: [PATCH] test(xcatd): the purity check misses exited() mutating in place The subtest composed the two calls as exited(forked($pace,10),11), so exited() only ever got forked()'s throwaway intermediate to mutate. An exited() that wrote in place left $pace untouched and the assertion stayed green -- the review that found this confirmed it by making exited() impure and watching the file pass. Check each on a state it was handed directly. Verified the other way round: making exited() assign into its argument and return the same reference now reddens both new assertions, where before it reddened nothing. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> --- xCAT-test/unit/xcatd_monitor_respawn.t | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/xCAT-test/unit/xcatd_monitor_respawn.t b/xCAT-test/unit/xcatd_monitor_respawn.t index fe7a36751..1912c8fbf 100644 --- a/xCAT-test/unit/xcatd_monitor_respawn.t +++ b/xCAT-test/unit/xcatd_monitor_respawn.t @@ -186,6 +186,24 @@ subtest 'the pacing functions are pure' => sub { is_deeply( $pace, \%before, 'exited()/forked() leave the state they were given alone' ); isnt( $after, $pace, 'they return a new state rather than the same reference' ); is( due( $pace, 0 ), due( $pace, 0 ), 'due() is free of side effects' ); + + # Each of the two has to be checked on a state it was handed DIRECTLY. Composing them as + # exited(forked($pace,...)) only ever lets exited() mutate forked()'s throwaway + # intermediate, so an exited() that wrote in place would leave $pace untouched and the + # assertion above green. + my $only_forked = xCAT::RespawnUtils::policy( min_interval => 1, max_interval => 8 ); + my %before_forked = %$only_forked; + my $forked_out = forked( $only_forked, 10 ); + is_deeply( $only_forked, \%before_forked, + 'forked() alone leaves the state it was given alone' ); + isnt( $forked_out, $only_forked, 'forked() returns a new state' ); + + my $only_exited = xCAT::RespawnUtils::policy( min_interval => 1, max_interval => 8 ); + my %before_exited = %$only_exited; + my $exited_out = exited( $only_exited, 5 ); + is_deeply( $only_exited, \%before_exited, + 'exited() alone leaves the state it was given alone' ); + isnt( $exited_out, $only_exited, 'exited() returns a new state' ); }; # --- the window the reaper looks through ------------------------------------