From 52379e37d3f56432b35819cc5faf2906909f1e78 Mon Sep 17 00:00:00 2001 From: Tom Clune Date: Tue, 10 Mar 2026 10:17:27 -0400 Subject: [PATCH 1/8] Add --shuffle and --seed options for randomizing test execution order Implements issue #530: Add command-line options to randomize test execution order within each test suite to help detect hidden test dependencies. Changes: - Add --shuffle flag to enable random test ordering - Add --seed=N option to specify random seed (0=time-based, implies --shuffle) - Implement Fisher-Yates shuffle algorithm in TestSuite - Tests are shuffled within each suite, preserving suite boundaries - Add comprehensive unit tests (Test_TestSuite_Shuffle.F90) - Add integration tests (ShuffleIntegrationTests.pf, test_shuffle.sh) All tests pass, including reproducibility with same seed and verification that shuffle actually changes test order. --- src/funit/FUnit.F90 | 31 ++- src/funit/core/TestSuite.F90 | 88 +++++++ tests/funit-core/CMakeLists.txt | 17 ++ tests/funit-core/ShuffleIntegrationTests.pf | 57 ++++ tests/funit-core/Test_TestSuite_Shuffle.F90 | 277 ++++++++++++++++++++ tests/funit-core/serial_tests.F90 | 2 + tests/funit-core/test_shuffle.sh | 109 ++++++++ 7 files changed, 579 insertions(+), 2 deletions(-) create mode 100644 tests/funit-core/ShuffleIntegrationTests.pf create mode 100644 tests/funit-core/Test_TestSuite_Shuffle.F90 create mode 100755 tests/funit-core/test_shuffle.sh diff --git a/src/funit/FUnit.F90 b/src/funit/FUnit.F90 index ff896676..779d6249 100644 --- a/src/funit/FUnit.F90 +++ b/src/funit/FUnit.F90 @@ -54,6 +54,8 @@ logical function generic_run(load_tests, context) result(status) type(ArgParser), target :: parser logical :: debug logical :: xml + logical :: shuffle + integer :: random_seed type (StringUnlimitedMap) :: options class(*), pointer :: option integer :: unit @@ -133,6 +135,24 @@ logical function generic_run(load_tests, context) result(status) call apply_exclude_filters(option, suite, unit) end if + ! Apply shuffle if requested + shuffle = .false. + option => options%at('shuffle') + if (associated(option)) then + call cast(option, shuffle) + end if + + random_seed = 0 + option => options%at('random_seed') + if (associated(option)) then + call cast(option, random_seed) + if (random_seed /= 0) shuffle = .true. ! Seed implies shuffle + end if + + if (shuffle) then + call suite%set_shuffle(random_seed) + end if + r = runner%run(suite, context) status = r%wasSuccessful() @@ -376,8 +396,15 @@ subroutine set_command_line_options() & dest='tap_file', action='store', default=0, & & help='add a TAP listener and send results to file name') - call parser%add_argument('-x', '--xml', action='store_true', & - & help='print results with XmlPrinter') + call parser%add_argument('-x', '--xml', action='store_true', & + & help='print results with XmlPrinter') + + call parser%add_argument('--shuffle', action='store_true', & + & help='randomize test execution order within each suite') + + call parser%add_argument('--seed', type='integer', & + & dest='random_seed', action='store', default=0, & + & help='random seed for test shuffling (0=time-based, implies --shuffle)') #ifndef _GNU options = parser%parse_args() diff --git a/src/funit/core/TestSuite.F90 b/src/funit/core/TestSuite.F90 index 62d54546..3fb673b0 100644 --- a/src/funit/core/TestSuite.F90 +++ b/src/funit/core/TestSuite.F90 @@ -37,6 +37,8 @@ module PF_TestSuite !!$ private character(:), allocatable :: name type (TestVector) :: tests + logical :: shuffle_enabled = .false. + integer :: shuffle_seed = 0 contains procedure :: getName procedure :: setName @@ -49,6 +51,8 @@ module PF_TestSuite procedure :: getTestCases procedure :: filter procedure :: filter_sub + procedure :: set_shuffle + procedure :: shuffle_tests end type TestSuite interface TestSuite @@ -77,6 +81,8 @@ recursive subroutine copy(this, b) this%name = b%name this%tests = b%tests + this%shuffle_enabled = b%shuffle_enabled + this%shuffle_seed = b%shuffle_seed end subroutine copy @@ -104,6 +110,11 @@ recursive subroutine run(this, tstResult, context) class (Test), pointer :: t integer :: i + ! Shuffle tests if enabled + if (this%shuffle_enabled) then + call this%shuffle_tests() + end if + do i = 1, this%tests%size() t => this%tests%at(i) call t%run(tstResult, context) @@ -217,4 +228,81 @@ recursive function filter(this, a_filter) result(new_suite) end function filter + subroutine set_shuffle(this, seed) + class(TestSuite), intent(inout) :: this + integer, intent(in) :: seed + + this%shuffle_enabled = .true. + this%shuffle_seed = seed + end subroutine set_shuffle + + + subroutine shuffle_tests(this) + class(TestSuite), intent(inout) :: this + integer :: i, j, n + integer, allocatable :: seed_array(:) + integer :: seed_size + real :: rnd + class(Test), allocatable :: temp_test + type(TestVector) :: shuffled_tests + type(TestReference), allocatable :: temp_array(:) + + n = this%tests%size() + if (n <= 1) return + + ! Initialize random seed + call random_seed(size=seed_size) + allocate(seed_array(seed_size)) + + if (this%shuffle_seed == 0) then + ! Time-based seed using system_clock + call system_clock(seed_array(1)) + if (seed_size > 1) then + ! Fill remaining with derived values + do i = 2, seed_size + seed_array(i) = seed_array(1) + i * 1000 + end do + end if + else + ! User-specified seed + seed_array(:) = this%shuffle_seed + end if + + call random_seed(put=seed_array) + + ! Fisher-Yates shuffle algorithm + ! Build shuffled vector by copying tests in random order + ! First, copy all tests to temp array for random access + allocate(temp_array(n)) + + do i = 1, n + allocate(temp_array(i)%pTest, source=this%tests%at(i)) + end do + + ! Now shuffle using Fisher-Yates + do i = n, 2, -1 + call random_number(rnd) + j = int(rnd * i) + 1 + if (i /= j) then + ! Swap temp_array(i) and temp_array(j) using move_alloc + call move_alloc(temp_array(i)%pTest, temp_test) + call move_alloc(temp_array(j)%pTest, temp_array(i)%pTest) + call move_alloc(temp_test, temp_array(j)%pTest) + end if + end do + + ! Rebuild tests vector in shuffled order + shuffled_tests = TestVector() + do i = 1, n + call shuffled_tests%push_back(temp_array(i)%pTest) + end do + + ! Replace original tests with shuffled tests + this%tests = shuffled_tests + + deallocate(temp_array) + deallocate(seed_array) + end subroutine shuffle_tests + + end module PF_TestSuite diff --git a/tests/funit-core/CMakeLists.txt b/tests/funit-core/CMakeLists.txt index 9a9ccf89..a23cf996 100644 --- a/tests/funit-core/CMakeLists.txt +++ b/tests/funit-core/CMakeLists.txt @@ -68,6 +68,7 @@ set (test_srcs Test_TestResult.F90 Test_XmlPrinter.F90 Test_TestSuite.F90 + Test_TestSuite_Shuffle.F90 ) if (OPENMP_FORTRAN_FOUND) @@ -144,5 +145,21 @@ set_tests_properties(command_line_filtering PROPERTIES DEPENDS filter_cmdline_tests.x ) +# Shuffle integration tests +add_pfunit_ctest (shuffle_int_tests.x + TEST_SOURCES ShuffleIntegrationTests.pf + LINK_LIBRARIES other_shared +) +add_dependencies(build-tests shuffle_int_tests.x) + +add_test(NAME shuffle_integration + COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/test_shuffle.sh + ${CMAKE_CURRENT_BINARY_DIR}/shuffle_int_tests.x + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} +) +set_tests_properties(shuffle_integration PROPERTIES + DEPENDS shuffle_int_tests.x +) + diff --git a/tests/funit-core/ShuffleIntegrationTests.pf b/tests/funit-core/ShuffleIntegrationTests.pf new file mode 100644 index 00000000..1fbf2534 --- /dev/null +++ b/tests/funit-core/ShuffleIntegrationTests.pf @@ -0,0 +1,57 @@ +module ShuffleIntegrationTests + use FUnit + implicit none + +contains + + @test + subroutine test_01() + @assertTrue(.true.) + end subroutine + + @test + subroutine test_02() + @assertTrue(.true.) + end subroutine + + @test + subroutine test_03() + @assertTrue(.true.) + end subroutine + + @test + subroutine test_04() + @assertTrue(.true.) + end subroutine + + @test + subroutine test_05() + @assertTrue(.true.) + end subroutine + + @test + subroutine test_06() + @assertTrue(.true.) + end subroutine + + @test + subroutine test_07() + @assertTrue(.true.) + end subroutine + + @test + subroutine test_08() + @assertTrue(.true.) + end subroutine + + @test + subroutine test_09() + @assertTrue(.true.) + end subroutine + + @test + subroutine test_10() + @assertTrue(.true.) + end subroutine + +end module ShuffleIntegrationTests diff --git a/tests/funit-core/Test_TestSuite_Shuffle.F90 b/tests/funit-core/Test_TestSuite_Shuffle.F90 new file mode 100644 index 00000000..7e0e718d --- /dev/null +++ b/tests/funit-core/Test_TestSuite_Shuffle.F90 @@ -0,0 +1,277 @@ +#include "unused_dummy.fh" + +!------------------------------------------------------------------------------- +! NASA/GSFC, Advanced Software Technology Group +!------------------------------------------------------------------------------- +! MODULE: Test_TestSuite_Shuffle +! +!> @brief +!! Unit tests for TestSuite shuffle functionality +!! +!! @author +!! Tom Clune, NASA/GSFC +!! +!! @date +!! 10 Mar 2025 +! +!------------------------------------------------------------------------------- +module Test_TestSuite_Shuffle + use PF_TestSuite, only: TestSuite + use PF_TestResult + use PF_Assert, only: assertEqual, assertNotEqual, assertTrue, assertFalse + use PF_TestMethod, only: TestMethod + use PF_SerialContext + implicit none + private + + public :: suite + + ! Internal mock for TestResult that logs test execution order + type, extends(TestResult) :: LoggingResult + character(len=200) :: log + contains + procedure :: run => logging_run + end type LoggingResult + +contains + + function suite() + type (TestSuite) :: suite + + suite = TestSuite('TestSuite_Shuffle_Suite') + + call suite%addTest(TestMethod('test_no_shuffle_deterministic', & + & test_no_shuffle_deterministic)) + call suite%addTest(TestMethod('test_shuffle_seed_reproducible', & + & test_shuffle_seed_reproducible)) + call suite%addTest(TestMethod('test_shuffle_changes_order', & + & test_shuffle_changes_order)) + call suite%addTest(TestMethod('test_shuffle_empty_suite', & + & test_shuffle_empty_suite)) + call suite%addTest(TestMethod('test_shuffle_single_test', & + & test_shuffle_single_test)) + call suite%addTest(TestMethod('test_shuffle_preserves_suite_boundaries', & + & test_shuffle_preserves_suite_boundaries)) + end function suite + + + subroutine test_no_shuffle_deterministic() + ! Verify that without shuffle, tests run in deterministic order + type(TestSuite) :: test_suite + type(LoggingResult) :: result1, result2 + + test_suite = TestSuite('test') + call test_suite%addTest(TestMethod('t1', dummy_method)) + call test_suite%addTest(TestMethod('t2', dummy_method)) + call test_suite%addTest(TestMethod('t3', dummy_method)) + call test_suite%addTest(TestMethod('t4', dummy_method)) + call test_suite%addTest(TestMethod('t5', dummy_method)) + + result1%TestResult = TestResult() + result1%log = '' + result2%TestResult = TestResult() + result2%log = '' + + call test_suite%run(result1, SerialContext()) + call test_suite%run(result2, SerialContext()) + + ! Both runs should produce identical order + call assertEqual(result1%log, result2%log, 'Order should be deterministic without shuffle') + end subroutine test_no_shuffle_deterministic + + + subroutine test_shuffle_seed_reproducible() + ! Verify that same seed produces same order + type(TestSuite) :: suite1, suite2 + type(LoggingResult) :: result1, result2 + character(len=200) :: order1, order2 + integer :: i, pos1, pos2 + + suite1 = TestSuite('test1') + call suite1%addTest(TestMethod('t1', dummy_method)) + call suite1%addTest(TestMethod('t2', dummy_method)) + call suite1%addTest(TestMethod('t3', dummy_method)) + call suite1%addTest(TestMethod('t4', dummy_method)) + call suite1%addTest(TestMethod('t5', dummy_method)) + call suite1%addTest(TestMethod('t6', dummy_method)) + call suite1%addTest(TestMethod('t7', dummy_method)) + call suite1%addTest(TestMethod('t8', dummy_method)) + call suite1%addTest(TestMethod('t9', dummy_method)) + call suite1%addTest(TestMethod('t10', dummy_method)) + + suite2 = TestSuite('test1') ! Use same suite name for easier comparison + call suite2%addTest(TestMethod('t1', dummy_method)) + call suite2%addTest(TestMethod('t2', dummy_method)) + call suite2%addTest(TestMethod('t3', dummy_method)) + call suite2%addTest(TestMethod('t4', dummy_method)) + call suite2%addTest(TestMethod('t5', dummy_method)) + call suite2%addTest(TestMethod('t6', dummy_method)) + call suite2%addTest(TestMethod('t7', dummy_method)) + call suite2%addTest(TestMethod('t8', dummy_method)) + call suite2%addTest(TestMethod('t9', dummy_method)) + call suite2%addTest(TestMethod('t10', dummy_method)) + + ! Set same seed for both + call suite1%set_shuffle(12345) + call suite2%set_shuffle(12345) + + result1%TestResult = TestResult() + result1%log = '' + result2%TestResult = TestResult() + result2%log = '' + + call suite1%run(result1, SerialContext()) + call suite2%run(result2, SerialContext()) + + ! Same seed should produce same order + call assertEqual(result1%log, result2%log, 'Same seed should produce same order') + end subroutine test_shuffle_seed_reproducible + + + subroutine test_shuffle_changes_order() + ! Verify that shuffle actually changes order + ! With 10 tests, probability of same order is 1/10! ≈ 0.0000003% + type(TestSuite) :: test_suite + type(LoggingResult) :: no_shuffle_result, shuffle_result + logical :: order_changed + + test_suite = TestSuite('test') + call test_suite%addTest(TestMethod('t1', dummy_method)) + call test_suite%addTest(TestMethod('t2', dummy_method)) + call test_suite%addTest(TestMethod('t3', dummy_method)) + call test_suite%addTest(TestMethod('t4', dummy_method)) + call test_suite%addTest(TestMethod('t5', dummy_method)) + call test_suite%addTest(TestMethod('t6', dummy_method)) + call test_suite%addTest(TestMethod('t7', dummy_method)) + call test_suite%addTest(TestMethod('t8', dummy_method)) + call test_suite%addTest(TestMethod('t9', dummy_method)) + call test_suite%addTest(TestMethod('t10', dummy_method)) + + ! Run without shuffle first + no_shuffle_result%TestResult = TestResult() + no_shuffle_result%log = '' + call test_suite%run(no_shuffle_result, SerialContext()) + + ! Now enable shuffle with a fixed seed and run again + call test_suite%set_shuffle(99999) + shuffle_result%TestResult = TestResult() + shuffle_result%log = '' + call test_suite%run(shuffle_result, SerialContext()) + + ! Order should be different + order_changed = (no_shuffle_result%log /= shuffle_result%log) + call assertTrue(order_changed, 'Shuffle should change test order') + end subroutine test_shuffle_changes_order + + + subroutine test_shuffle_empty_suite() + ! Verify shuffle doesn't crash on empty suite + type(TestSuite) :: test_suite + type(LoggingResult) :: result + + test_suite = TestSuite('empty') + call test_suite%set_shuffle(12345) + + result%TestResult = TestResult() + result%log = '' + + ! Should not crash + call test_suite%run(result, SerialContext()) + + call assertEqual('', result%log, 'Empty suite should have empty log') + end subroutine test_shuffle_empty_suite + + + subroutine test_shuffle_single_test() + ! Verify shuffle works correctly with single test + type(TestSuite) :: test_suite + type(LoggingResult) :: result + + test_suite = TestSuite('single') + call test_suite%addTest(TestMethod('only_test', dummy_method)) + call test_suite%set_shuffle(12345) + + result%TestResult = TestResult() + result%log = '' + + call test_suite%run(result, SerialContext()) + + call assertTrue(index(result%log, 'only_test') > 0, 'Single test should run') + end subroutine test_shuffle_single_test + + + subroutine test_shuffle_preserves_suite_boundaries() + ! Verify that shuffle preserves suite boundaries (no inter-suite mixing) + type(TestSuite) :: parent_suite, childA, childB + type(LoggingResult) :: result + character(len=200) :: log_str + integer :: pos_a1, pos_a2, pos_b1, pos_b2 + + parent_suite = TestSuite('parent') + + childA = TestSuite('childA') + call childA%addTest(TestMethod('a1', dummy_method)) + call childA%addTest(TestMethod('a2', dummy_method)) + call childA%addTest(TestMethod('a3', dummy_method)) + + childB = TestSuite('childB') + call childB%addTest(TestMethod('b1', dummy_method)) + call childB%addTest(TestMethod('b2', dummy_method)) + call childB%addTest(TestMethod('b3', dummy_method)) + + call parent_suite%addTest(childA) + call parent_suite%addTest(childB) + + ! Enable shuffle on parent (note: current implementation only shuffles + ! tests within each suite, not the suites themselves) + call parent_suite%set_shuffle(54321) + + result%TestResult = TestResult() + result%log = '' + + call parent_suite%run(result, SerialContext()) + log_str = result%log + + ! All childA tests should appear before all childB tests + ! (or vice versa if suites are shuffled, but tests within suites shouldn't mix) + pos_a1 = index(log_str, 'childA.a1') + pos_a2 = index(log_str, 'childA.a2') + pos_b1 = index(log_str, 'childB.b1') + pos_b2 = index(log_str, 'childB.b2') + + ! Verify all tests were found + call assertTrue(pos_a1 > 0, 'childA.a1 should be in log') + call assertTrue(pos_a2 > 0, 'childA.a2 should be in log') + call assertTrue(pos_b1 > 0, 'childB.b1 should be in log') + call assertTrue(pos_b2 > 0, 'childB.b2 should be in log') + + ! Either all A's before all B's, or all B's before all A's + if (pos_a1 < pos_b1) then + ! A's should all come before B's + call assertTrue(pos_a2 < pos_b1, 'Suite boundaries should be preserved (A before B)') + else + ! B's should all come before A's + call assertTrue(pos_b2 < pos_a1, 'Suite boundaries should be preserved (B before A)') + end if + end subroutine test_shuffle_preserves_suite_boundaries + + + ! Helper methods + subroutine dummy_method() + ! Empty test method + end subroutine dummy_method + + + recursive subroutine logging_run(this, test, context) + use PF_TestCase + use PF_SurrogateTestCase + use PF_ParallelContext + class (LoggingResult), intent(inout) :: this + class (SurrogateTestCase), intent(inout) :: test + class (ParallelContext), intent(in) :: context + + _UNUSED_DUMMY(context) + this%log = trim(this%log)//' ::'//trim(test%getName()) + end subroutine logging_run + +end module Test_TestSuite_Shuffle diff --git a/tests/funit-core/serial_tests.F90 b/tests/funit-core/serial_tests.F90 index 2d40130d..d028c745 100644 --- a/tests/funit-core/serial_tests.F90 +++ b/tests/funit-core/serial_tests.F90 @@ -26,6 +26,7 @@ logical function runTests() result(success) use Test_TestResult, only: testResultSuite => suite ! (6) use Test_TestSuite, only: testTestSuiteSuite => suite ! (7) + use Test_TestSuite_Shuffle, only: testTestSuiteShuffleSuite => suite ! (7b) use Test_TestMethod, only: testTestMethodSuite => suite ! (8) use Test_SimpleTestCase, only: testSimpleTestCaseSuite => suite ! (9) @@ -56,6 +57,7 @@ logical function runTests() result(success) ADD(testResultSuite) ADD(testTestSuiteSuite) + ADD(testTestSuiteShuffleSuite) ADD(testTestMethodSuite) ADD(testSimpleTestCaseSuite) diff --git a/tests/funit-core/test_shuffle.sh b/tests/funit-core/test_shuffle.sh new file mode 100755 index 00000000..31025f9b --- /dev/null +++ b/tests/funit-core/test_shuffle.sh @@ -0,0 +1,109 @@ +#!/bin/bash +# Integration test for --shuffle and --seed command-line options + +set -e # Exit on error + +TEST_EXE="$1" +if [ -z "$TEST_EXE" ] || [ ! -x "$TEST_EXE" ]; then + echo "ERROR: Test executable not provided or not executable: $TEST_EXE" + exit 1 +fi + +# Color output helpers +RED='\033[0;31m' +GREEN='\033[0;32m' +NC='\033[0m' # No Color + +FAILED=0 +PASSED=0 + +# Test function +run_test() { + local description="$1" + shift + + echo -n "Testing: $description ... " + + # Run test and capture output + if "$TEST_EXE" "$@" > /dev/null 2>&1; then + echo -e "${GREEN}PASS${NC}" + PASSED=$((PASSED + 1)) + return 0 + else + echo -e "${RED}FAIL${NC}" + FAILED=$((FAILED + 1)) + return 1 + fi +} + +echo "==========================================" +echo "Testing pFUnit --shuffle and --seed Options" +echo "==========================================" +echo "" + +# Test 1: No shuffle - should run successfully +run_test "No shuffle (baseline)" + +# Test 2: With --shuffle - should run successfully +run_test "With --shuffle flag" --shuffle + +# Test 3: With --seed - should run successfully +run_test "With --seed=12345" --seed=12345 + +# Test 4: Verify --seed=N gives reproducible order +echo -n "Testing: Seed reproducibility ... " +output1=$("$TEST_EXE" --seed=54321 2>&1) +output2=$("$TEST_EXE" --seed=54321 2>&1) +if [ "$output1" = "$output2" ]; then + echo -e "${GREEN}PASS${NC}" + PASSED=$((PASSED + 1)) +else + echo -e "${RED}FAIL${NC} (outputs differ with same seed)" + FAILED=$((FAILED + 1)) +fi + +# Test 5: Verify multiple runs with different seeds all succeed +# Note: We can't easily verify order changes from output since pFUnit +# doesn't print test names in normal mode. The unit tests verify the +# actual shuffling behavior. +echo -n "Testing: Different seeds all run successfully ... " +success_count=0 +for seed in 111 222 333 444 555; do + if "$TEST_EXE" --seed=$seed > /dev/null 2>&1; then + success_count=$((success_count + 1)) + fi +done +if [ $success_count -eq 5 ]; then + echo -e "${GREEN}PASS${NC}" + PASSED=$((PASSED + 1)) +else + echo -e "${RED}FAIL${NC} (only $success_count/5 runs succeeded)" + FAILED=$((FAILED + 1)) +fi + +# Test 6: Verify --shuffle works multiple times +echo -n "Testing: Multiple --shuffle runs complete successfully ... " +success_count=0 +for i in {1..5}; do + if "$TEST_EXE" --shuffle > /dev/null 2>&1; then + success_count=$((success_count + 1)) + fi +done +if [ $success_count -eq 5 ]; then + echo -e "${GREEN}PASS${NC}" + PASSED=$((PASSED + 1)) +else + echo -e "${RED}FAIL${NC} (only $success_count/5 runs succeeded)" + FAILED=$((FAILED + 1)) +fi + +echo "" +echo "==========================================" +echo "Results: $PASSED passed, $FAILED failed" +echo "==========================================" + +if [ $FAILED -gt 0 ]; then + exit 1 +fi + +exit 0 From fe826a61d93e443d44b58bd979a933f3dc0aea2b Mon Sep 17 00:00:00 2001 From: Tom Clune Date: Tue, 10 Mar 2026 10:29:56 -0400 Subject: [PATCH 2/8] Refactor shuffle_tests to reduce nesting Extract helper subroutines to improve code readability: - initialize_random_seed(): Handles seed initialization logic - swap_tests(): Encapsulates test swapping with move_alloc This reduces maximum nesting level from 4 to 2 and makes each subroutine have a single, clear responsibility. --- src/funit/core/TestSuite.F90 | 69 ++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 35 deletions(-) diff --git a/src/funit/core/TestSuite.F90 b/src/funit/core/TestSuite.F90 index 3fb673b0..31eef213 100644 --- a/src/funit/core/TestSuite.F90 +++ b/src/funit/core/TestSuite.F90 @@ -240,8 +240,6 @@ end subroutine set_shuffle subroutine shuffle_tests(this) class(TestSuite), intent(inout) :: this integer :: i, j, n - integer, allocatable :: seed_array(:) - integer :: seed_size real :: rnd class(Test), allocatable :: temp_test type(TestVector) :: shuffled_tests @@ -250,45 +248,19 @@ subroutine shuffle_tests(this) n = this%tests%size() if (n <= 1) return - ! Initialize random seed - call random_seed(size=seed_size) - allocate(seed_array(seed_size)) - - if (this%shuffle_seed == 0) then - ! Time-based seed using system_clock - call system_clock(seed_array(1)) - if (seed_size > 1) then - ! Fill remaining with derived values - do i = 2, seed_size - seed_array(i) = seed_array(1) + i * 1000 - end do - end if - else - ! User-specified seed - seed_array(:) = this%shuffle_seed - end if + call initialize_random_seed(this%shuffle_seed) - call random_seed(put=seed_array) - - ! Fisher-Yates shuffle algorithm - ! Build shuffled vector by copying tests in random order - ! First, copy all tests to temp array for random access + ! Copy tests to temp array for random access allocate(temp_array(n)) - do i = 1, n allocate(temp_array(i)%pTest, source=this%tests%at(i)) end do - ! Now shuffle using Fisher-Yates + ! Fisher-Yates shuffle do i = n, 2, -1 call random_number(rnd) j = int(rnd * i) + 1 - if (i /= j) then - ! Swap temp_array(i) and temp_array(j) using move_alloc - call move_alloc(temp_array(i)%pTest, temp_test) - call move_alloc(temp_array(j)%pTest, temp_array(i)%pTest) - call move_alloc(temp_test, temp_array(j)%pTest) - end if + if (i /= j) call swap_tests(temp_array(i)%pTest, temp_array(j)%pTest, temp_test) end do ! Rebuild tests vector in shuffled order @@ -297,12 +269,39 @@ subroutine shuffle_tests(this) call shuffled_tests%push_back(temp_array(i)%pTest) end do - ! Replace original tests with shuffled tests this%tests = shuffled_tests - deallocate(temp_array) - deallocate(seed_array) end subroutine shuffle_tests + subroutine initialize_random_seed(user_seed) + integer, intent(in) :: user_seed + integer, allocatable :: seed_array(:) + integer :: seed_size, i + + call random_seed(size=seed_size) + allocate(seed_array(seed_size)) + + if (user_seed == 0) then + call system_clock(seed_array(1)) + do i = 2, seed_size + seed_array(i) = seed_array(1) + i * 1000 + end do + else + seed_array(:) = user_seed + end if + + call random_seed(put=seed_array) + deallocate(seed_array) + end subroutine initialize_random_seed + + + subroutine swap_tests(test_a, test_b, temp) + class(Test), allocatable, intent(inout) :: test_a, test_b, temp + call move_alloc(test_a, temp) + call move_alloc(test_b, test_a) + call move_alloc(temp, test_b) + end subroutine swap_tests + + end module PF_TestSuite From a8b1a3f94a6a0c4e94fcbeff2c98dbe3d7c9825b Mon Sep 17 00:00:00 2001 From: Tom Clune Date: Tue, 10 Mar 2026 10:32:13 -0400 Subject: [PATCH 3/8] Simplify shuffle by shuffling indices instead of test objects Replace complex array-based shuffling with simpler index-based approach: - Shuffle integer indices array instead of Test objects - Build new vector by accessing tests in shuffled index order - Eliminates TestReference type, temp array allocations, and swap_tests() This is cleaner, more efficient (no polymorphic object copying), and easier to understand. --- src/funit/core/TestSuite.F90 | 49 +++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/src/funit/core/TestSuite.F90 b/src/funit/core/TestSuite.F90 index 31eef213..21c93f35 100644 --- a/src/funit/core/TestSuite.F90 +++ b/src/funit/core/TestSuite.F90 @@ -239,38 +239,30 @@ end subroutine set_shuffle subroutine shuffle_tests(this) class(TestSuite), intent(inout) :: this - integer :: i, j, n - real :: rnd - class(Test), allocatable :: temp_test + integer :: i, n + integer, allocatable :: indices(:) type(TestVector) :: shuffled_tests - type(TestReference), allocatable :: temp_array(:) n = this%tests%size() if (n <= 1) return call initialize_random_seed(this%shuffle_seed) - ! Copy tests to temp array for random access - allocate(temp_array(n)) + ! Create shuffled index array + allocate(indices(n)) do i = 1, n - allocate(temp_array(i)%pTest, source=this%tests%at(i)) - end do - - ! Fisher-Yates shuffle - do i = n, 2, -1 - call random_number(rnd) - j = int(rnd * i) + 1 - if (i /= j) call swap_tests(temp_array(i)%pTest, temp_array(j)%pTest, temp_test) + indices(i) = i end do + call shuffle_indices(indices) - ! Rebuild tests vector in shuffled order + ! Build new vector in shuffled order shuffled_tests = TestVector() do i = 1, n - call shuffled_tests%push_back(temp_array(i)%pTest) + call shuffled_tests%push_back(this%tests%at(indices(i))) end do this%tests = shuffled_tests - deallocate(temp_array) + deallocate(indices) end subroutine shuffle_tests @@ -296,12 +288,23 @@ subroutine initialize_random_seed(user_seed) end subroutine initialize_random_seed - subroutine swap_tests(test_a, test_b, temp) - class(Test), allocatable, intent(inout) :: test_a, test_b, temp - call move_alloc(test_a, temp) - call move_alloc(test_b, test_a) - call move_alloc(temp, test_b) - end subroutine swap_tests + subroutine shuffle_indices(indices) + integer, intent(inout) :: indices(:) + integer :: i, j, n, temp + real :: rnd + + n = size(indices) + ! Fisher-Yates shuffle + do i = n, 2, -1 + call random_number(rnd) + j = int(rnd * i) + 1 + if (i /= j) then + temp = indices(i) + indices(i) = indices(j) + indices(j) = temp + end if + end do + end subroutine shuffle_indices end module PF_TestSuite From d6b7b8f6c030d258ce0690505ade64fd31c8e66f Mon Sep 17 00:00:00 2001 From: Tom Clune Date: Tue, 10 Mar 2026 10:35:16 -0400 Subject: [PATCH 4/8] Clean up shuffle implementation per code review - Use array constructor indices = [(i, i=1, n)] instead of explicit loop - Remove superfluous explicit deallocate() for allocatable arrays - Eliminate else clause by setting default seed_array first - Replace magic number 1000 with 997 (prime) and add explanatory comment --- src/funit/core/TestSuite.F90 | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/funit/core/TestSuite.F90 b/src/funit/core/TestSuite.F90 index 21c93f35..d1c510dc 100644 --- a/src/funit/core/TestSuite.F90 +++ b/src/funit/core/TestSuite.F90 @@ -249,10 +249,7 @@ subroutine shuffle_tests(this) call initialize_random_seed(this%shuffle_seed) ! Create shuffled index array - allocate(indices(n)) - do i = 1, n - indices(i) = i - end do + indices = [(i, i=1, n)] call shuffle_indices(indices) ! Build new vector in shuffled order @@ -262,7 +259,6 @@ subroutine shuffle_tests(this) end do this%tests = shuffled_tests - deallocate(indices) end subroutine shuffle_tests @@ -274,17 +270,17 @@ subroutine initialize_random_seed(user_seed) call random_seed(size=seed_size) allocate(seed_array(seed_size)) + seed_array(:) = user_seed if (user_seed == 0) then + ! Time-based seed - use system clock for first element + ! and derive remaining elements to ensure variety call system_clock(seed_array(1)) do i = 2, seed_size - seed_array(i) = seed_array(1) + i * 1000 + seed_array(i) = seed_array(1) + i * 997 ! Prime offset for variety end do - else - seed_array(:) = user_seed end if call random_seed(put=seed_array) - deallocate(seed_array) end subroutine initialize_random_seed From 87a329eca8b8e856f7cfa73dc5211dbd0cf69d73 Mon Sep 17 00:00:00 2001 From: Tom Clune Date: Tue, 10 Mar 2026 10:38:14 -0400 Subject: [PATCH 5/8] Simplify random seed initialization When user_seed==0, just use Fortran's default random_seed() initialization instead of manually constructing a time-based seed. The compiler's default is perfectly adequate for non-reproducible randomization. --- src/funit/core/TestSuite.F90 | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/funit/core/TestSuite.F90 b/src/funit/core/TestSuite.F90 index d1c510dc..2dbefda1 100644 --- a/src/funit/core/TestSuite.F90 +++ b/src/funit/core/TestSuite.F90 @@ -265,22 +265,18 @@ end subroutine shuffle_tests subroutine initialize_random_seed(user_seed) integer, intent(in) :: user_seed integer, allocatable :: seed_array(:) - integer :: seed_size, i + integer :: seed_size - call random_seed(size=seed_size) - allocate(seed_array(seed_size)) - - seed_array(:) = user_seed if (user_seed == 0) then - ! Time-based seed - use system clock for first element - ! and derive remaining elements to ensure variety - call system_clock(seed_array(1)) - do i = 2, seed_size - seed_array(i) = seed_array(1) + i * 997 ! Prime offset for variety - end do + ! Use compiler's default random initialization + call random_seed() + else + ! Use user-provided seed + call random_seed(size=seed_size) + allocate(seed_array(seed_size)) + seed_array(:) = user_seed + call random_seed(put=seed_array) end if - - call random_seed(put=seed_array) end subroutine initialize_random_seed From 8df70ec5416a5dd13f1401f9282c7c06f4874df7 Mon Sep 17 00:00:00 2001 From: Tom Clune Date: Tue, 10 Mar 2026 10:42:00 -0400 Subject: [PATCH 6/8] Simplify initialize_random_seed with spread() and early return --- src/funit/core/TestSuite.F90 | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/funit/core/TestSuite.F90 b/src/funit/core/TestSuite.F90 index 2dbefda1..71e43b17 100644 --- a/src/funit/core/TestSuite.F90 +++ b/src/funit/core/TestSuite.F90 @@ -270,13 +270,13 @@ subroutine initialize_random_seed(user_seed) if (user_seed == 0) then ! Use compiler's default random initialization call random_seed() - else - ! Use user-provided seed - call random_seed(size=seed_size) - allocate(seed_array(seed_size)) - seed_array(:) = user_seed - call random_seed(put=seed_array) + return end if + + ! Use user-provided seed + call random_seed(size=seed_size) + seed_array = spread(user_seed, 1, seed_size) + call random_seed(put=seed_array) end subroutine initialize_random_seed From 95c3c802affaebcd87d70f9d95cc196021031450 Mon Sep 17 00:00:00 2001 From: Tom Clune Date: Tue, 10 Mar 2026 10:43:48 -0400 Subject: [PATCH 7/8] Reduce nesting in shuffle_indices with early cycle --- src/funit/core/TestSuite.F90 | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/funit/core/TestSuite.F90 b/src/funit/core/TestSuite.F90 index 71e43b17..5cd5fdf8 100644 --- a/src/funit/core/TestSuite.F90 +++ b/src/funit/core/TestSuite.F90 @@ -290,11 +290,10 @@ subroutine shuffle_indices(indices) do i = n, 2, -1 call random_number(rnd) j = int(rnd * i) + 1 - if (i /= j) then - temp = indices(i) - indices(i) = indices(j) - indices(j) = temp - end if + if (i == j) cycle + temp = indices(i) + indices(i) = indices(j) + indices(j) = temp end do end subroutine shuffle_indices From a0acabbf32c319d81b1dd314e294aa63851688f3 Mon Sep 17 00:00:00 2001 From: Tom Clune Date: Tue, 10 Mar 2026 10:45:16 -0400 Subject: [PATCH 8/8] Update ChangeLog for shuffle feature --- ChangeLog.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ChangeLog.md b/ChangeLog.md index bdad312d..84035c07 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -5,6 +5,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Test shuffling support to detect hidden test dependencies (issue #530) + - `--shuffle` flag to randomize test execution order within each suite + - `--seed=N` option to specify random seed for reproducibility (0 uses time-based seed, implies --shuffle) + - Fisher-Yates shuffle algorithm implemented in `TestSuite` module + - Both unit tests and integration tests included + ## [4.16.0] - 2026-02-23 ### Added