From 302ba0a7f79f31c400ba39f87e2f20f2be754d37 Mon Sep 17 00:00:00 2001 From: jhendersonHDF Date: Mon, 7 Oct 2024 12:27:09 -0500 Subject: [PATCH] Minor refactoring of testframe.c testing framework (#4930) Added setup and cleanup callback parameters to TestInit() to perform setup and cleanup tasks once for whole test program Removed TestCleanup() function since its functionality is covered by PerformTests() Added check of the HDF5_NOCLEANUP environment variable in GetTestCleanup() --- c++/test/testhdf5.cpp | 8 +-- test/testframe.c | 139 ++++++++++++++++++++++++------------------ test/testframe.h | 46 +++++++------- test/testhdf5.c | 17 +++--- test/ttsafe.c | 17 +++--- testpar/t_2Gio.c | 14 ++++- testpar/t_shapesame.c | 15 ++++- testpar/testphdf5.c | 15 ++++- 8 files changed, 161 insertions(+), 110 deletions(-) diff --git a/c++/test/testhdf5.cpp b/c++/test/testhdf5.cpp index bbbe7b6a400..1df73baaf19 100644 --- a/c++/test/testhdf5.cpp +++ b/c++/test/testhdf5.cpp @@ -32,8 +32,6 @@ PerformTests() -- Perform requested testing GetTestSummary() -- Retrieve Summary request value TestSummary() -- Display test summary - GetTestCleanup() -- Retrieve Cleanup request value - TestCleanup() -- Clean up files from testing GetTestNumErrs() -- Retrieve the number of testing errors ***************************************************************************/ @@ -57,7 +55,7 @@ main(int argc, char *argv[]) // caused deliberately and expected. Exception::dontPrint(); /* Initialize testing framework */ - TestInit(argv[0], NULL, NULL, 0); + TestInit(argv[0], NULL, NULL, NULL, NULL, 0); // testing file creation and opening in tfile.cpp AddTest("tfile", test_file, NULL, cleanup_file, NULL, 0, "File I/O Operations"); @@ -112,10 +110,6 @@ main(int argc, char *argv[]) if (GetTestSummary()) TestSummary(stdout); - /* Clean up test files, if allowed */ - if (GetTestCleanup() && !getenv(HDF5_NOCLEANUP)) - TestCleanup(); - /* Release test infrastructure */ TestShutdown(); diff --git a/test/testframe.c b/test/testframe.c index dd5a7bb8dce..a630a179b9e 100644 --- a/test/testframe.c +++ b/test/testframe.c @@ -43,6 +43,7 @@ static unsigned TestCount = 0; /* Number of tests currently added to test static const char *TestProgName = NULL; static void (*TestPrivateUsage_g)(FILE *stream) = NULL; static herr_t (*TestPrivateParser_g)(int argc, char *argv[]) = NULL; +static herr_t (*TestCleanupFunc_g)(void) = NULL; static int TestNumErrs_g = 0; /* Total number of errors that occurred for whole test program */ static bool TestEnableErrorStack = true; /* Whether to show error stacks from the library */ @@ -65,21 +66,25 @@ AddTest(const char *TestName, void (*TestFunc)(const void *), void (*TestSetupFu void *new_test_data = NULL; if (*TestName == '\0') { - fprintf(stderr, "%s: empty string given for test name\n", __func__); + if (TestFrameworkProcessID_g == 0) + fprintf(stderr, "%s: empty string given for test name\n", __func__); return FAIL; } if (strlen(TestName) >= MAXTESTNAME) { - fprintf(stderr, "%s: test name ('%s') too long, increase MAXTESTNAME(%d).\n", __func__, TestName, - MAXTESTNAME); + if (TestFrameworkProcessID_g == 0) + fprintf(stderr, "%s: test name ('%s') too long, increase MAXTESTNAME(%d).\n", __func__, TestName, + MAXTESTNAME); return FAIL; } if (strlen(TestDescr) >= MAXTESTDESC) { - fprintf(stderr, "%s: test description ('%s') too long, increase MAXTESTDESC(%d).\n", __func__, - TestDescr, MAXTESTDESC); + if (TestFrameworkProcessID_g == 0) + fprintf(stderr, "%s: test description ('%s') too long, increase MAXTESTDESC(%d).\n", __func__, + TestDescr, MAXTESTDESC); return FAIL; } if ((TestData && (0 == TestDataSize)) || (!TestData && (0 != TestDataSize))) { - fprintf(stderr, "%s: invalid test data size (%zu)\n", __func__, TestDataSize); + if (TestFrameworkProcessID_g == 0) + fprintf(stderr, "%s: invalid test data size (%zu)\n", __func__, TestDataSize); return FAIL; } @@ -89,9 +94,10 @@ AddTest(const char *TestName, void (*TestFunc)(const void *), void (*TestSetupFu unsigned newAlloc = MAX(1, TestAlloc * 2); if (NULL == (newTest = realloc(TestArray, newAlloc * sizeof(TestStruct)))) { - fprintf(stderr, - "%s: couldn't reallocate test array, TestCount = %u, TestAlloc = %u, newAlloc = %u\n", - __func__, TestCount, TestAlloc, newAlloc); + if (TestFrameworkProcessID_g == 0) + fprintf(stderr, + "%s: couldn't reallocate test array, TestCount = %u, TestAlloc = %u, newAlloc = %u\n", + __func__, TestCount, TestAlloc, newAlloc); return FAIL; } @@ -113,7 +119,8 @@ AddTest(const char *TestName, void (*TestFunc)(const void *), void (*TestSetupFu /* Make a copy of the additional test data given */ if (TestData) { if (NULL == (new_test_data = malloc(TestDataSize))) { - fprintf(stderr, "%s: couldn't allocate space for additional test data\n", __func__); + if (TestFrameworkProcessID_g == 0) + fprintf(stderr, "%s: couldn't allocate space for additional test data\n", __func__); return FAIL; } @@ -136,12 +143,14 @@ AddTest(const char *TestName, void (*TestFunc)(const void *), void (*TestSetupFu */ herr_t TestInit(const char *ProgName, void (*TestPrivateUsage)(FILE *stream), - herr_t (*TestPrivateParser)(int argc, char *argv[]), int TestProcessID) + herr_t (*TestPrivateParser)(int argc, char *argv[]), herr_t (*TestSetupFunc)(void), + herr_t (*TestCleanupFunc)(void), int TestProcessID) { /* Turn off automatic error reporting if requested */ if (!TestEnableErrorStack) { if (H5Eset_auto2(H5E_DEFAULT, NULL, NULL) < 0) { - fprintf(stderr, "%s: can't disable error stack\n", __func__); + if (TestFrameworkProcessID_g == 0) + fprintf(stderr, "%s: can't disable error stack\n", __func__); return FAIL; } } @@ -155,7 +164,9 @@ TestInit(const char *ProgName, void (*TestPrivateUsage)(FILE *stream), TestPrivateUsage_g = TestPrivateUsage; if (NULL != TestPrivateParser) TestPrivateParser_g = TestPrivateParser; + TestCleanupFunc_g = TestCleanupFunc; + /* Set process ID for later use */ TestFrameworkProcessID_g = TestProcessID; /* Set/reset global variables from h5test that may be used by @@ -166,6 +177,13 @@ TestInit(const char *ProgName, void (*TestPrivateUsage)(FILE *stream), n_tests_failed_g = 0; n_tests_skipped_g = 0; + /* Call test framework setup callback if provided */ + if (TestSetupFunc && TestSetupFunc() < 0) { + if (TestFrameworkProcessID_g == 0) + fprintf(stderr, "%s: error occurred in test framework initialization callback\n", __func__); + return FAIL; + } + return SUCCEED; } @@ -337,21 +355,24 @@ TestParseCmdLine(int argc, char *argv[]) errno = 0; max_threads = strtol(*argv, NULL, 10); if (errno != 0) { - fprintf(stderr, - "error while parsing value (%s) specified for maximum number of threads\n", - *argv); + if (TestFrameworkProcessID_g == 0) + fprintf(stderr, + "error while parsing value (%s) specified for maximum number of threads\n", + *argv); ret_value = FAIL; goto done; } if (max_threads <= 0) { - fprintf(stderr, "invalid value (%ld) specified for maximum number of threads\n", - max_threads); + if (TestFrameworkProcessID_g == 0) + fprintf(stderr, "invalid value (%ld) specified for maximum number of threads\n", + max_threads); ret_value = FAIL; goto done; } else if (max_threads > (long)INT_MAX) { - fprintf(stderr, "value (%ld) specified for maximum number of threads too large\n", - max_threads); + if (TestFrameworkProcessID_g == 0) + fprintf(stderr, "value (%ld) specified for maximum number of threads too large\n", + max_threads); ret_value = FAIL; goto done; } @@ -387,24 +408,23 @@ TestParseCmdLine(int argc, char *argv[]) /* * Execute all tests that aren't being skipped */ -void +herr_t PerformTests(void) { for (unsigned Loop = 0; Loop < TestCount; Loop++) { int old_num_errs = TestNumErrs_g; if (TestArray[Loop].TestSkipFlag) { - if (TestFrameworkProcessID_g == 0) - MESSAGE(2, ("Skipping -- %s (%s) \n", TestArray[Loop].Description, TestArray[Loop].Name)); + MESSAGE(2, ("Skipping -- %s (%s) \n", TestArray[Loop].Description, TestArray[Loop].Name)); continue; } - if (TestFrameworkProcessID_g == 0) { - MESSAGE(2, ("Testing -- %s (%s) \n", TestArray[Loop].Description, TestArray[Loop].Name)); - MESSAGE(5, ("===============================================\n")); - } + MESSAGE(2, ("Testing -- %s (%s) \n", TestArray[Loop].Description, TestArray[Loop].Name)); + MESSAGE(5, ("===============================================\n")); - TestAlarmOn(); + if (TestAlarmOn() < 0) + MESSAGE(5, ("Couldn't enable test alarm timer for test -- %s (%s) \n", + TestArray[Loop].Description, TestArray[Loop].Name)); if (TestArray[Loop].TestSetupFunc) TestArray[Loop].TestSetupFunc(TestArray[Loop].TestParameters); @@ -418,19 +438,17 @@ PerformTests(void) TestArray[Loop].TestNumErrors = TestNumErrs_g - old_num_errs; - if (TestFrameworkProcessID_g == 0) { - MESSAGE(5, ("===============================================\n")); - MESSAGE(5, ("There were %d errors detected.\n\n", TestArray[Loop].TestNumErrors)); - } + MESSAGE(5, ("===============================================\n")); + MESSAGE(5, ("There were %d errors detected.\n\n", TestArray[Loop].TestNumErrors)); } - if (TestFrameworkProcessID_g == 0) { - MESSAGE(2, ("\n\n")); - if (TestNumErrs_g) - MESSAGE(VERBO_NONE, ("!!! %d Error(s) were detected !!!\n\n", TestNumErrs_g)); - else - MESSAGE(VERBO_NONE, ("All tests were successful. \n\n")); - } + MESSAGE(2, ("\n\n")); + if (TestNumErrs_g) + MESSAGE(VERBO_NONE, ("!!! %d Error(s) were detected !!!\n\n", TestNumErrs_g)); + else + MESSAGE(VERBO_NONE, ("All tests were successful. \n\n")); + + return SUCCEED; } /* @@ -497,31 +515,26 @@ TestSummary(FILE *stream) fprintf(stream, "\n\n"); } -/* - * Perform test cleanup - */ -void -TestCleanup(void) -{ - if (TestFrameworkProcessID_g == 0) - MESSAGE(2, ("\nCleaning Up temp files...\n\n")); - - for (unsigned Loop = 0; Loop < TestCount; Loop++) - if (!TestArray[Loop].TestSkipFlag && TestArray[Loop].TestCleanupFunc != NULL) - TestArray[Loop].TestCleanupFunc(TestArray[Loop].TestParameters); -} - /* * Shutdown the test infrastructure */ -void +herr_t TestShutdown(void) { + /* Clean up test state first before tearing down testing framework */ + if (TestCleanupFunc_g && TestCleanupFunc_g() < 0) { + if (TestFrameworkProcessID_g == 0) + fprintf(stderr, "%s: error occurred in test framework initialization callback\n", __func__); + return FAIL; + } + if (TestArray) for (unsigned Loop = 0; Loop < TestCount; Loop++) free(TestArray[Loop].TestParameters); free(TestArray); + + return SUCCEED; } /* @@ -585,6 +598,12 @@ GetTestSummary(void) H5_ATTR_PURE bool GetTestCleanup(void) { + /* Don't cleanup files if the HDF5_NOCLEANUP environment + * variable is defined to anything + */ + if (getenv(HDF5_NOCLEANUP)) + SetTestNoCleanup(); + return TestDoCleanUp_g; } @@ -615,8 +634,9 @@ ParseTestVerbosity(char *argv) errno = 0; verb_level = strtol(argv, NULL, 10); if (errno != 0) { - fprintf(stderr, "%s: error while parsing value (%s) specified for test verbosity\n", __func__, - argv); + if (TestFrameworkProcessID_g == 0) + fprintf(stderr, "%s: error while parsing value (%s) specified for test verbosity\n", __func__, + argv); return FAIL; } @@ -770,12 +790,15 @@ TestAlarmOn(void) errno = 0; alarm_sec = strtoul(env_val, NULL, 10); if (errno != 0) { - fprintf(stderr, "%s: error while parsing value (%s) specified for alarm timeout\n", __func__, - env_val); + if (TestFrameworkProcessID_g == 0) + fprintf(stderr, "%s: error while parsing value (%s) specified for alarm timeout\n", __func__, + env_val); return FAIL; } else if (alarm_sec > (unsigned long)UINT_MAX) { - fprintf(stderr, "%s: value (%lu) specified for alarm timeout too large\n", __func__, alarm_sec); + if (TestFrameworkProcessID_g == 0) + fprintf(stderr, "%s: value (%lu) specified for alarm timeout too large\n", __func__, + alarm_sec); return FAIL; } } diff --git a/test/testframe.h b/test/testframe.h index d730c6e0958..21ffcfe673f 100644 --- a/test/testframe.h +++ b/test/testframe.h @@ -137,6 +137,11 @@ extern "C" { * \param[in] TestPrivateParser Pointer to a function which parses * command-line arguments which are specific to * the test program + * \param[in] TestSetupFunc Pointer to a function which will be called + * as part of TestInit() + * \param[in] TestCleanupFunc Pointer to a function which will be called + * when the testing framework is being shut + * down * \param[in] TestProcessID ID for the process calling TestInit(). Used * to control printing of output in parallel * test programs. @@ -167,6 +172,18 @@ extern "C" { * standard list of arguments it recognizes. \p TestPrivateParser * may be NULL. * + * \p TestSetupFunc is a pointer to a function that can be used to + * setup any state needed before tests begin executing. If provided, + * this callback function will be called as part of TestInit() once + * the testing framework has been fully initialized. \p TestSetupFunc + * may be NULL. + * + * \p TestCleanupFunc is a pointer to a function that can be used + * to clean up any state after tests have finished executing. If + * provided, this callback function will be called by TestShutdown() + * before the testing framework starts being shut down. + * \p TestCleanupFunc may be NULL. + * * \p TestProcessID is an integer value that is used to distinguish * between processes when multiple are involved in running a test * program. This is primarily useful for controlling testing @@ -180,7 +197,8 @@ extern "C" { * */ H5TEST_DLL herr_t TestInit(const char *ProgName, void (*TestPrivateUsage)(FILE *stream), - int (*TestPrivateParser)(int argc, char *argv[]), int TestProcessID); + int (*TestPrivateParser)(int argc, char *argv[]), herr_t (*TestSetupFunc)(void), + herr_t (*TestCleanupFunc)(void), int TestProcessID); /** * -------------------------------------------------------------------------- @@ -188,7 +206,7 @@ H5TEST_DLL herr_t TestInit(const char *ProgName, void (*TestPrivateUsage)(FILE * * * \brief Shuts down the testing framework * - * \return void + * \return \herr_t * * \details TestShutdown() shuts down the testing framework by tearing down * the internal state needed for running tests and freeing any @@ -199,7 +217,7 @@ H5TEST_DLL herr_t TestInit(const char *ProgName, void (*TestPrivateUsage)(FILE * * \see TestInit() * */ -H5TEST_DLL void TestShutdown(void); +H5TEST_DLL herr_t TestShutdown(void); /** * -------------------------------------------------------------------------- @@ -366,7 +384,7 @@ H5TEST_DLL herr_t TestParseCmdLine(int argc, char *argv[]); * \brief Executes all tests added by AddTest() that aren't flagged to be * skipped * - * \return void + * \return \herr_t * * \details PerformTests() runs all tests that aren't flagged to be skipped * in the order added by calls to AddTest(). For each test, the @@ -380,7 +398,7 @@ H5TEST_DLL herr_t TestParseCmdLine(int argc, char *argv[]); * \see AddTest(), TestAlarmOn() * */ -H5TEST_DLL void PerformTests(void); +H5TEST_DLL herr_t PerformTests(void); /** * -------------------------------------------------------------------------- @@ -404,24 +422,6 @@ H5TEST_DLL void PerformTests(void); */ H5TEST_DLL void TestSummary(FILE *stream); -/** - * -------------------------------------------------------------------------- - * \ingroup H5TEST - * - * \brief Calls the 'cleanup' callback for each test added to the list of - * tests - * - * \return void - * - * \details TestCleanup() performs cleanup by calling the 'cleanup' callback - * for each test added to the lists of tests, as long as the test - * isn't flagged to be skipped. - * - * \see SetTestCleanup() - * - */ -H5TEST_DLL void TestCleanup(void); - /** * -------------------------------------------------------------------------- * \ingroup H5TEST diff --git a/test/testhdf5.c b/test/testhdf5.c index 0df372858dc..925b15981e0 100644 --- a/test/testhdf5.c +++ b/test/testhdf5.c @@ -47,7 +47,7 @@ main(int argc, char *argv[]) H5Pclose(fapl_id); /* Initialize testing framework */ - if (TestInit(argv[0], NULL, NULL, 0) < 0) { + if (TestInit(argv[0], NULL, NULL, NULL, NULL, 0) < 0) { fprintf(stderr, "couldn't initialize testing framework\n"); exit(EXIT_FAILURE); } @@ -90,18 +90,21 @@ main(int argc, char *argv[]) } /* Perform requested testing */ - PerformTests(); + if (PerformTests() < 0) { + fprintf(stderr, "couldn't run tests\n"); + TestShutdown(); + exit(EXIT_FAILURE); + } /* Display test summary, if requested */ if (GetTestSummary()) TestSummary(stdout); - /* Clean up test files, if allowed */ - if (GetTestCleanup() && !getenv(HDF5_NOCLEANUP)) - TestCleanup(); - /* Release test infrastructure */ - TestShutdown(); + if (TestShutdown() < 0) { + fprintf(stderr, "couldn't shut down testing framework\n"); + exit(EXIT_FAILURE); + } /* Exit failure if errors encountered; else exit success. */ /* No need to print anything since PerformTests() already does. */ diff --git a/test/ttsafe.c b/test/ttsafe.c index 75adca40556..b1cf42f4e0a 100644 --- a/test/ttsafe.c +++ b/test/ttsafe.c @@ -97,7 +97,7 @@ main(int argc, char *argv[]) { /* Initialize testing framework */ - if (TestInit(argv[0], NULL, NULL, 0) < 0) { + if (TestInit(argv[0], NULL, NULL, NULL, NULL, 0) < 0) { fprintf(stderr, "couldn't initialize testing framework\n"); return -1; } @@ -184,18 +184,21 @@ main(int argc, char *argv[]) } /* Perform requested testing */ - PerformTests(); + if (PerformTests() < 0) { + fprintf(stderr, "couldn't run tests\n"); + TestShutdown(); + return -1; + } /* Display test summary, if requested */ if (GetTestSummary()) TestSummary(stdout); - /* Clean up test files, if allowed */ - if (GetTestCleanup() && !getenv(HDF5_NOCLEANUP)) - TestCleanup(); - /* Release test infrastructure */ - TestShutdown(); + if (TestShutdown() < 0) { + fprintf(stderr, "couldn't shut down testing framework\n"); + return -1; + } return GetTestNumErrs(); diff --git a/testpar/t_2Gio.c b/testpar/t_2Gio.c index fca14d72a70..3a5a63d11cc 100644 --- a/testpar/t_2Gio.c +++ b/testpar/t_2Gio.c @@ -2536,7 +2536,7 @@ main(int argc, char **argv) /* Initialize testing framework */ if (mpi_rank < 2) { - if (TestInit(argv[0], usage, parse_options, mpi_rank) < 0) { + if (TestInit(argv[0], usage, parse_options, NULL, NULL, mpi_rank) < 0) { fprintf(stderr, "couldn't initialize testing framework\n"); MPI_Abort(MPI_COMM_WORLD, -1); } @@ -2581,7 +2581,11 @@ main(int argc, char **argv) H5Pset_fapl_mpio(fapl, test_comm, MPI_INFO_NULL); /* Perform requested testing */ - PerformTests(); + if (PerformTests() < 0) { + fprintf(stderr, "couldn't run tests\n"); + TestShutdown(); + MPI_Abort(MPI_COMM_WORLD, -1); + } } MPI_Barrier(MPI_COMM_WORLD); @@ -2604,6 +2608,12 @@ main(int argc, char **argv) filenames[i] = NULL; } + if (TestShutdown() < 0) { + if (MAINPROCESS) + fprintf(stderr, "couldn't shut down testing framework\n"); + MPI_Abort(MPI_COMM_WORLD, -1); + } + H5close(); if (test_comm != MPI_COMM_WORLD) { MPI_Comm_free(&test_comm); diff --git a/testpar/t_shapesame.c b/testpar/t_shapesame.c index ee226e64c0a..d841e7e9269 100644 --- a/testpar/t_shapesame.c +++ b/testpar/t_shapesame.c @@ -4335,7 +4335,7 @@ main(int argc, char **argv) } /* Initialize testing framework */ - if (TestInit(argv[0], usage, parse_options, mpi_rank) < 0) { + if (TestInit(argv[0], usage, parse_options, NULL, NULL, mpi_rank) < 0) { if (MAINPROCESS) { fprintf(stderr, "couldn't initialize testing framework\n"); fflush(stderr); @@ -4388,7 +4388,12 @@ main(int argc, char **argv) } /* Perform requested testing */ - PerformTests(); + if (PerformTests() < 0) { + if (MAINPROCESS) + fprintf(stderr, "couldn't run tests\n"); + TestShutdown(); + MPI_Abort(MPI_COMM_WORLD, -1); + } /* make sure all processes are finished before final report, cleanup * and exit. @@ -4430,7 +4435,11 @@ main(int argc, char **argv) H5close(); /* Release test infrastructure */ - TestShutdown(); + if (TestShutdown() < 0) { + if (MAINPROCESS) + fprintf(stderr, "couldn't shut down testing framework\n"); + MPI_Abort(MPI_COMM_WORLD, -1); + } MPI_Finalize(); diff --git a/testpar/testphdf5.c b/testpar/testphdf5.c index 51dc3012c5b..2f30e722323 100644 --- a/testpar/testphdf5.c +++ b/testpar/testphdf5.c @@ -335,7 +335,7 @@ main(int argc, char **argv) VRFY((H5Pget_vol_cap_flags(fapl, &vol_cap_flags_g) >= 0), "H5Pget_vol_cap_flags succeeded"); /* Initialize testing framework */ - if (TestInit(argv[0], usage, parse_options, mpi_rank) < 0) { + if (TestInit(argv[0], usage, parse_options, NULL, NULL, mpi_rank) < 0) { if (MAINPROCESS) fprintf(stderr, "couldn't initialize testing framework\n"); MPI_Finalize(); @@ -569,7 +569,12 @@ main(int argc, char **argv) } /* Perform requested testing */ - PerformTests(); + if (PerformTests() < 0) { + if (MAINPROCESS) + fprintf(stderr, "couldn't run tests\n"); + TestShutdown(); + MPI_Abort(MPI_COMM_WORLD, -1); + } /* make sure all processes are finished before final report, cleanup * and exit. @@ -611,7 +616,11 @@ main(int argc, char **argv) H5close(); /* Release test infrastructure */ - TestShutdown(); + if (TestShutdown() < 0) { + if (MAINPROCESS) + fprintf(stderr, "couldn't shut down testing framework\n"); + MPI_Abort(MPI_COMM_WORLD, -1); + } /* MPI_Finalize must be called AFTER H5close which may use MPI calls */ MPI_Finalize();