From d3cb92dd6e31f7201783c5c1acddfb5d5c0effd7 Mon Sep 17 00:00:00 2001 From: Anderson Chauphan Date: Mon, 25 Nov 2024 18:20:10 -0600 Subject: [PATCH 1/8] Add PR script argument to skip packageEnables.cmake creation Add argument that can be passed downstream to skip the creation of the packageEnables.cmake file to the main entry point of our PR script, PullRequestLinuxDriverTest.py. Signed-off-by: Anderson Chauphan --- packages/framework/pr_tools/PullRequestLinuxDriverTest.py | 7 +++++++ .../pr_tools/unittests/test_PullRequestLinuxDriverTest.py | 1 + 2 files changed, 8 insertions(+) diff --git a/packages/framework/pr_tools/PullRequestLinuxDriverTest.py b/packages/framework/pr_tools/PullRequestLinuxDriverTest.py index 73caa76a1c62..7fa3724dfdc1 100755 --- a/packages/framework/pr_tools/PullRequestLinuxDriverTest.py +++ b/packages/framework/pr_tools/PullRequestLinuxDriverTest.py @@ -196,6 +196,13 @@ def parse_args(): default=default_filename_packageenables, help="{} Default={}".format(desc_package_enables, default_filename_packageenables)) + optional.add_argument('--skip-create-packageenables', + dest="skip_create_packageenables", + action="store_true", + help="Skip the creation of the packageEnables.cmake fragment file generated by " + \ + "the TriBITS infrastructure indicating which packages are to be enabled based on file " + \ + "changes between a source and target branch. Default=") + desc_subprojects_file = "The subprojects_file is used by the testing infrastructure. This parameter " + \ "allows the default, generated file, to be overridden. Generally this should " + \ "not be changed from the defaults." diff --git a/packages/framework/pr_tools/unittests/test_PullRequestLinuxDriverTest.py b/packages/framework/pr_tools/unittests/test_PullRequestLinuxDriverTest.py index 91e8e7068d71..a87993834afc 100755 --- a/packages/framework/pr_tools/unittests/test_PullRequestLinuxDriverTest.py +++ b/packages/framework/pr_tools/unittests/test_PullRequestLinuxDriverTest.py @@ -80,6 +80,7 @@ def setUp(self): workspace_dir='/dev/null/Trilinos_clone', filename_packageenables='../packageEnables.cmake', filename_subprojects='../package_subproject_list.cmake', + skip_create_packageenables=False, test_mode='standard', req_mem_per_core=3.0, max_cores_allowed=12, From 0d8c4dc65c2907a0546ba61baba9d49cf8d4d411 Mon Sep 17 00:00:00 2001 From: Anderson Chauphan Date: Mon, 25 Nov 2024 18:22:08 -0600 Subject: [PATCH 2/8] Specify argument to skip packageEnables creation in AT1 AutoTester1 begins its PR script from LaunchDriver.sh -> PullRequestLinuxDriver.sh where arguments are propogated to the main PR script, PullRequestLinuxDriverTest.py. Signed-off-by: Anderson Chauphan --- packages/framework/pr_tools/PullRequestLinuxDriver.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/framework/pr_tools/PullRequestLinuxDriver.sh b/packages/framework/pr_tools/PullRequestLinuxDriver.sh index 01679dae2cdb..391be53a7e75 100755 --- a/packages/framework/pr_tools/PullRequestLinuxDriver.sh +++ b/packages/framework/pr_tools/PullRequestLinuxDriver.sh @@ -270,6 +270,11 @@ then test_cmd_options+=( "--use-explicit-cachefile ") fi +if [[ ${GENCONFIG_BUILD_NAME} == *"framework"* ]] +then + test_cmd_options+=( "--skip-create-packageenables ") +fi + test_cmd="${PYTHON_EXE:?} ${REPO_ROOT:?}/packages/framework/pr_tools/PullRequestLinuxDriverTest.py ${test_cmd_options[@]}" # Call the script to launch the tests From 0ac49bf74fca7f633fbfc4fc44fbd5b91eeccb56 Mon Sep 17 00:00:00 2001 From: Anderson Chauphan Date: Mon, 25 Nov 2024 19:27:55 -0600 Subject: [PATCH 3/8] Add logic to skip create_package_enables_file Added the logic that skips the creation of the packageEnables.cmake file in Trilinos containing all the packages with changes that need to be enabled for PR testing. The implementation of current packageEnables generation writes to a hard-coded file that does not take into consideration the existing `filename_packageenables` argument. Further changes should be made such that the creation of the packageEnables file depends on the value of the `filename_packageenables` instead of this `skip-create-packageenables` flag, but here we are. Signed-off-by: Anderson Chauphan --- .../TrilinosPRConfigurationBase.py | 31 +++++++++++++------ .../test_TrilinosPRConfigurationBase.py | 17 ++++++++++ ...est_TrilinosPRConfigurationInstallation.py | 1 + .../test_TrilinosPRConfigurationStandard.py | 1 + 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/packages/framework/pr_tools/trilinosprhelpers/TrilinosPRConfigurationBase.py b/packages/framework/pr_tools/trilinosprhelpers/TrilinosPRConfigurationBase.py index 5ca1499c2dfa..02b3a9f22fe6 100644 --- a/packages/framework/pr_tools/trilinosprhelpers/TrilinosPRConfigurationBase.py +++ b/packages/framework/pr_tools/trilinosprhelpers/TrilinosPRConfigurationBase.py @@ -267,6 +267,13 @@ def arg_filename_packageenables(self): """ return self.args.filename_packageenables + @property + def arg_skip_create_packageenables(self): + """ + This property controls whether the creation of a packageEnables.cmake fragment file + should be skipped. + """ + return self.args.skip_create_packageenables @property def arg_workspace_dir(self): @@ -501,7 +508,7 @@ def pullrequest_build_name(self): elif self.arg_dashboard_build_name != "__UNKNOWN__": output = self.arg_dashboard_build_name else: - output = self.arg_pr_genconfig_job_name + output = self.arg_pr_genconfig_job_name return output @@ -799,16 +806,22 @@ def prepare_test(self): self.message("| E N V I R O N M E N T S E T U P C O M P L E T E") self.message("+" + "-"*68 + "+") - self.message("+" + "-"*68 + "+") - self.message("| G e n e r a t e `packageEnables.cmake` S T A R T I N G") - self.message("+" + "-"*68 + "+") + if self.arg_skip_create_packageenables: + self.message("+" + "-"*68 + "+") + self.message("| S K I P P I N G `packageEnables.cmake` G E N E R A T I O N") + self.message("+" + "-"*68 + "+") - self.create_package_enables_file(dryrun=self.args.dry_run) + else: + self.message("+" + "-"*68 + "+") + self.message("| G e n e r a t e `packageEnables.cmake` S T A R T I N G") + self.message("+" + "-"*68 + "+") - self.message("+" + "-"*68 + "+") - self.message("| G e n e r a t e `packageEnables.cmake` C O M P L E T E D") - self.message("+" + "-"*68 + "+") - self.message("") + self.create_package_enables_file(dryrun=self.args.dry_run) + + self.message("+" + "-"*68 + "+") + self.message("| G e n e r a t e `packageEnables.cmake` C O M P L E T E D") + self.message("+" + "-"*68 + "+") + self.message("") return 0 diff --git a/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationBase.py b/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationBase.py index b323abfe1943..da3d44a382da 100755 --- a/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationBase.py +++ b/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationBase.py @@ -225,6 +225,7 @@ def dummy_args(self): ctest_drop_site="testing.sandia.gov", filename_packageenables="../packageEnables.cmake", filename_subprojects="../package_subproject_list.cmake", + skip_create_packageenables=False, mode="standard", req_mem_per_core=3.0, max_cores_allowed=12, @@ -697,6 +698,22 @@ def test_TrilinosPRConfigurationBase_prepare_test(self): self.assertEqual(ret, 0) + def test_TrilinosPRConfigurationBase_prepare_test_skip_create_package_enables_file(self): + """ + Test that the prepare_test method does not call the member function create_package_enables_file + when skip_create_packageenables is True + """ + args = self.dummy_args() + args.skip_create_packageenables = True + pr_config = trilinosprhelpers.TrilinosPRConfigurationBase(args) + + with patch('trilinosprhelpers.TrilinosPRConfigurationBase.create_package_enables_file') as m_call: + pr_config.prepare_test() + + expected_call_count = 0 + self.assertEqual(m_call.call_count, expected_call_count) + + def test_TrilinosPRConfigurationBase_prepare_test_FAIL(self): """ Test the prepare_test method where it would fail due to diff --git a/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationInstallation.py b/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationInstallation.py index 4eac6b0ceeda..551a57aff301 100755 --- a/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationInstallation.py +++ b/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationInstallation.py @@ -168,6 +168,7 @@ def dummy_args(self): ctest_drop_site="testint.sandia.gov", filename_packageenables="../packageEnables.cmake", filename_subprojects="../package_subproject_list.cmake", + skip_create_packageenables=False, mode="standard", req_mem_per_core=3.0, max_cores_allowed=12, diff --git a/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationStandard.py b/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationStandard.py index 9a722b30cce8..47586711a32d 100755 --- a/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationStandard.py +++ b/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationStandard.py @@ -167,6 +167,7 @@ def dummy_args(self): ctest_drop_site="testing.sandia.gov", filename_packageenables="../packageEnables.cmake", filename_subprojects="../package_subproject_list.cmake", + skip_create_packageenables=False, mode="standard", req_mem_per_core=3.0, max_cores_allowed=12, From 899922222d5c7bac8a46e236dc4a32a62b7245f2 Mon Sep 17 00:00:00 2001 From: Anderson Chauphan Date: Mon, 25 Nov 2024 19:37:13 -0600 Subject: [PATCH 4/8] Add --skip-create-packageenables arg to AT2 Framework tests Add argument to skip the creation of the packageEnables.cmake fragment file for the Framework AT2 job. This job should not run any other tests than Framework unittests, which was what it was doing before due to the packageEnables.cmake file always being generated along with the test being launched from the ctest-driver. Signed-off-by: Anderson Chauphan --- .github/workflows/AT2.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/AT2.yml b/.github/workflows/AT2.yml index 66d648bcd716..f9c0107293bd 100644 --- a/.github/workflows/AT2.yml +++ b/.github/workflows/AT2.yml @@ -384,7 +384,7 @@ jobs: --ctest-driver /home/runner/_work/Trilinos/Trilinos/cmake/SimpleTesting/cmake/ctest-driver.cmake \ --ctest-drop-site sems-cdash-son.sandia.gov/cdash \ --filename-subprojects ./package_subproject_list.cmake \ - --filename-packageenables ./packageEnables.cmake \ + --skip-create-packageenables \ - name: Summary if: ${{ !cancelled() }} shell: bash -l {0} From a6ee4029fc560a2385f7f3408b5bd99b47f40695 Mon Sep 17 00:00:00 2001 From: Anderson Chauphan Date: Wed, 4 Dec 2024 17:16:46 -0600 Subject: [PATCH 5/8] Explicitly define default case for "store_true" argparse Argparse "store_true" action implicitly defaults to false. Explicitly define false as the default case and update the help message. Signed-off-by: Anderson Chauphan --- packages/framework/pr_tools/PullRequestLinuxDriverTest.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/framework/pr_tools/PullRequestLinuxDriverTest.py b/packages/framework/pr_tools/PullRequestLinuxDriverTest.py index 7fa3724dfdc1..40f63e275df1 100755 --- a/packages/framework/pr_tools/PullRequestLinuxDriverTest.py +++ b/packages/framework/pr_tools/PullRequestLinuxDriverTest.py @@ -199,9 +199,10 @@ def parse_args(): optional.add_argument('--skip-create-packageenables', dest="skip_create_packageenables", action="store_true", + default=False, help="Skip the creation of the packageEnables.cmake fragment file generated by " + \ "the TriBITS infrastructure indicating which packages are to be enabled based on file " + \ - "changes between a source and target branch. Default=") + "changes between a source and target branch. Default=False") desc_subprojects_file = "The subprojects_file is used by the testing infrastructure. This parameter " + \ "allows the default, generated file, to be overridden. Generally this should " + \ From 0e996a775c866230ac5c4e1edc21d96398386378 Mon Sep 17 00:00:00 2001 From: Anderson Chauphan Date: Wed, 4 Dec 2024 17:17:16 -0600 Subject: [PATCH 6/8] Add creation of empty packageEnables and subprojects Add the creation of empty packageEnables.cmake and package_subproject_list.cmake files when `--skip-create-packageenables` argument is passed. This is due to SimpleTesting CTest drivers requiring these files to exist. packageEnables.cmake and package_subproject_list.cmake are created by get-changed-trilinos-packages.sh script which we do not always want to generate for cases such as Framework test line, where we do not want to spend extra compute resources building packages from changed files. We let the other PR builds handle that. Signed-off-by: Anderson Chauphan --- .../trilinosprhelpers/TrilinosPRConfigurationStandard.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/framework/pr_tools/trilinosprhelpers/TrilinosPRConfigurationStandard.py b/packages/framework/pr_tools/trilinosprhelpers/TrilinosPRConfigurationStandard.py index 401824ea8b6d..69f1ad653523 100644 --- a/packages/framework/pr_tools/trilinosprhelpers/TrilinosPRConfigurationStandard.py +++ b/packages/framework/pr_tools/trilinosprhelpers/TrilinosPRConfigurationStandard.py @@ -54,6 +54,15 @@ def execute_test(self): if not self.args.dry_run: gc.write_cmake_fragment() + if self.arg_skip_create_packageenables: + print("Optional --skip_create_packageenables found. " + + "Creating dummy packageEnables.cmake and package_subproject_list.cmake " + + "for CTest drivers.") + with open(self.arg_filename_packageenables, 'w'): + pass + with open(self.arg_filename_subprojects, 'w'): + pass + # Execute the call to ctest. verbosity_flag = "-VV" if "BUILD_NUMBER" in os.environ: From cd9ef979bbb18d1375b9f2b58e8bbbad28078c96 Mon Sep 17 00:00:00 2001 From: Anderson Date: Thu, 5 Dec 2024 14:00:37 -0700 Subject: [PATCH 7/8] Fix skip_create_package_enables test Using patch for the desired member function on the version of anaconda used on our AT1 Framework build lined did not work as it could not find the `create_package_enables_file` attribute in `TrilinosPRConfigurationBase`. Loading an older sems-anaconda version allowed the behavior and found the `trilinosprhelpers.TrilinosPRConfigurationBase.create_package_enables_file` attribute. In order to keep using our current version of aue/anaconda, use the implemented work around for mocking the member function. Signed-off-by: Anderson --- .../unittests/test_TrilinosPRConfigurationBase.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationBase.py b/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationBase.py index da3d44a382da..ffce49219b97 100755 --- a/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationBase.py +++ b/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationBase.py @@ -707,11 +707,10 @@ def test_TrilinosPRConfigurationBase_prepare_test_skip_create_package_enables_fi args.skip_create_packageenables = True pr_config = trilinosprhelpers.TrilinosPRConfigurationBase(args) - with patch('trilinosprhelpers.TrilinosPRConfigurationBase.create_package_enables_file') as m_call: - pr_config.prepare_test() + trilinosprhelpers.TrilinosPRConfigurationBase.create_package_enables_file = Mock() + pr_config.prepare_test() - expected_call_count = 0 - self.assertEqual(m_call.call_count, expected_call_count) + pr_config.create_package_enables_file.assert_not_called() def test_TrilinosPRConfigurationBase_prepare_test_FAIL(self): From 49e08eb04dd302f464500485bbab48de6bbeefba Mon Sep 17 00:00:00 2001 From: Anderson Chauphan Date: Thu, 5 Dec 2024 16:54:31 -0600 Subject: [PATCH 8/8] Re-scope mocked method Mocked a method at too large of scope and it affected other tests. Signed-off-by: Anderson Chauphan --- .../pr_tools/trilinosprhelpers/TrilinosPRConfigurationBase.py | 2 +- .../unittests/test_TrilinosPRConfigurationBase.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/framework/pr_tools/trilinosprhelpers/TrilinosPRConfigurationBase.py b/packages/framework/pr_tools/trilinosprhelpers/TrilinosPRConfigurationBase.py index 02b3a9f22fe6..616b23861268 100644 --- a/packages/framework/pr_tools/trilinosprhelpers/TrilinosPRConfigurationBase.py +++ b/packages/framework/pr_tools/trilinosprhelpers/TrilinosPRConfigurationBase.py @@ -630,7 +630,6 @@ def create_package_enables_file(self, dryrun=False): job_name = self.arg_pr_jenkins_job_name enable_map_entry = self.get_multi_property_from_config("ENABLE_MAP", job_name, delimeter=" ") - # Generate files using ATDM/TriBiTS Scripts if enable_map_entry is None: cmd = [os.path.join( self.arg_workspace_dir, @@ -741,6 +740,7 @@ def prepare_test(self): self.message("--- arg_ctest_driver = {}".format(self.arg_ctest_driver)) self.message("--- arg_ctest_drop_site = {}".format(self.arg_ctest_drop_site)) self.message("--- arg_ccache_enable = {}".format(self.arg_ccache_enable)) + self.message("--- arg_skip_create_packageenables = {}".format(self.arg_skip_create_packageenables)) self.message("") self.message("--- concurrency_build = {}".format(self.concurrency_build)) self.message("--- concurrency_test = {}".format(self.concurrency_test)) diff --git a/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationBase.py b/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationBase.py index ffce49219b97..b5673ff8901d 100755 --- a/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationBase.py +++ b/packages/framework/pr_tools/trilinosprhelpers/unittests/test_TrilinosPRConfigurationBase.py @@ -707,7 +707,7 @@ def test_TrilinosPRConfigurationBase_prepare_test_skip_create_package_enables_fi args.skip_create_packageenables = True pr_config = trilinosprhelpers.TrilinosPRConfigurationBase(args) - trilinosprhelpers.TrilinosPRConfigurationBase.create_package_enables_file = Mock() + pr_config.create_package_enables_file = Mock() pr_config.prepare_test() pr_config.create_package_enables_file.assert_not_called()