wait_for_tests: cdash Refactor and restore commit/time upload #4922

jgfouca · 2026-01-29T22:49:39Z

Description

Two main things. First, refactor the code to use tempfile temp dirs. These will be removed once wait_for_tests is done and will live in /tmp if possible instead of polluting pwd. Second, with recent cdash updates, we were no longer able to see commit and test time. This restores that info by putting it in notes.

Checklist

My code follows the style guidelines of this project (black formatting)
I have performed a self-review of my own code
My changes generate no new warnings
I have added tests that exercise my feature/fix and existing tests continue to pass
I have commented my code, particularly in hard-to-understand areas
I have made corresponding additions and changes to the documentation

Copilot

Pull request overview

This PR refactors the wait_for_tests.py module to use temporary directories for CDash-related operations and restores commit/time information to CDash reports via notes files.

Changes:

Refactored code to use tempfile.TemporaryDirectory for temporary storage instead of creating directories in the current working directory
Moved git commit and timing information from XML attributes to a separate notes file that is uploaded to CDash
Updated path handling to use pathlib.Path objects throughout the refactored code

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-29T22:52:55Z

CIME/wait_for_tests.py

+    prefixes = [None, first_result_case, os.getcwd()]
+    for prefix in prefixes:
+        try:
+            with tempfile.TemporaryDirectory(prefix=prefix) as tmpdir:


The parameter name is incorrect. tempfile.TemporaryDirectory accepts a dir parameter to specify the directory location, not prefix. The prefix parameter is used for the temporary directory name prefix (a string that will be prepended to the generated directory name), not for specifying where to create the directory. This should be tempfile.TemporaryDirectory(dir=prefix) instead of tempfile.TemporaryDirectory(prefix=prefix).

Suggested change

with tempfile.TemporaryDirectory(prefix=prefix) as tmpdir:

with tempfile.TemporaryDirectory(dir=prefix) as tmpdir:

Copilot · 2026-01-29T22:52:56Z

CIME/wait_for_tests.py

+                # Make notes file
+                with notes_file.open(mode="w") as notes_fd:
+                    notes_fd.write(
+                        f"Commit {git_commit}\nTotal testing time {time_info} seconds\n"


When time_info is "unknown" (a string), the notes file will output "Total testing time unknown seconds" which is grammatically incorrect. Consider changing line 494 to handle this case more gracefully, for example: f"Commit {git_commit}\nTotal testing time: {time_info if isinstance(time_info, str) else f'{time_info} seconds'}\n" or set time_info as a string like "unknown" or "X seconds" consistently.

Suggested change

f"Commit {git_commit}\nTotal testing time {time_info} seconds\n"

f"Commit {git_commit}\n"

f"Total testing time: "

f"{time_info if isinstance(time_info, str) else f'{time_info} seconds'}\n"

Copilot · 2026-01-29T22:52:56Z

CIME/wait_for_tests.py

-            hostname,
-            force_log_upload,
-        )
+                    stat, out, _ = run_cmd(


The function run_cmd is used here but it is not imported. The import statement at line 10 only imports run_cmd_no_fail, but not run_cmd. This will cause a NameError at runtime when this code path is executed. You need to add run_cmd to the imports from CIME.utils.

Copilot · 2026-01-29T22:52:56Z

CIME/wait_for_tests.py

-            logging.info("Upload SUCCESS:\n{}".format(out))
-            return
+        except Exception as e:
+            logging.info(f"Prexix '{prefix}' failed with error {e}")


There's a typo in the logging message: "Prexix" should be "Prefix".

Suggested change

logging.info(f"Prexix '{prefix}' failed with error {e}")

logging.info(f"Prefix '{prefix}' failed with error {e}")

Copilot · 2026-01-29T22:52:56Z

CIME/wait_for_tests.py

+                                "./xmlquery {} --value".format(param),
+                                from_dir=case_dir,
+                            )
+                        except:


Using a bare except clause is a bad practice as it catches all exceptions including SystemExit and KeyboardInterrupt. Consider catching specific exceptions (e.g., CIMEError, subprocess errors) or at minimum use 'except Exception:' to avoid catching system-level exceptions.

Suggested change

except:

except Exception:

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-30T18:21:01Z

CIME/wait_for_tests.py


    if cdash_build_name:
-        create_cdash_xml(
+        tmpdir = create_cdash_xml(


The create_cdash_xml function does not return a value, so this assignment will result in tmpdir = None. The variable tmpdir is not used afterward, so this assignment appears to be vestigial code. Consider removing the assignment and just call the function directly, or if the return value is intended for future use, update create_cdash_xml to return the temporary directory path.

Suggested change

tmpdir = create_cdash_xml(

create_cdash_xml(

Copilot · 2026-01-30T18:21:01Z

CIME/wait_for_tests.py

+                    notes_fd.write(
+                        f"Commit {git_commit}\nTotal testing time {time_info} seconds\n"


When time_info is "unknown" (line 477), the notes file will contain "Total testing time unknown seconds" which is grammatically awkward. Consider formatting this as either "Total testing time: unknown" or "Total testing time: {time_info} seconds" with conditional formatting to handle the "unknown" case more gracefully.

Suggested change

notes_fd.write(

f"Commit {git_commit}\nTotal testing time {time_info} seconds\n"

if time_info == "unknown":

total_time_line = "Total testing time: unknown"

else:

total_time_line = f"Total testing time {time_info} seconds"

notes_fd.write(

f"Commit {git_commit}\n{total_time_line}\n"

Copilot · 2026-01-30T18:21:01Z

CIME/wait_for_tests.py

+                    dart_config = """
+    SourceDirectory: {0}
+    BuildDirectory: {0}
+
+    # Site is something like machine.domain, i.e. pragmatic.crd
+    Site: {1}
+
+    # Build name is osname-revision-compiler, i.e. Linux-2.4.2-2smp-c++
+    BuildName: {2}
+
+    # Submission information
+    IsCDash: TRUE
+    CDashVersion:
+    QueryCDashVersion:
+    DropSite: my.cdash.org
+    DropLocation: /submit.php?project={3}
+    DropSiteUser:
+    DropSitePassword:
+    DropSiteMode:
+    DropMethod: {6}
+    TriggerSite:
+    ScpCommand: {4}
+
+    # Dashboard start time
+    NightlyStartTime: {5} UTC
+
+    UseLaunchers:
+    CurlOptions: CURLOPT_SSL_VERIFYPEER_OFF;CURLOPT_SSL_VERIFYHOST_OFF


The dart_config string now has 4 spaces of leading whitespace on each line (lines 533-559) compared to the original code. This indentation may cause issues if the DartConfiguration.tcl file parser does not handle leading whitespace correctly. Verify that the CDash/CTest configuration file format accepts this indentation, or remove the leading spaces from the string literal.

Copilot · 2026-01-30T18:21:02Z

CIME/wait_for_tests.py

+        except Exception as e:
+            logging.info(f"Temp dir '{tmpdir}' failed with error {e}")


The exception handler catches all exceptions with a broad Exception clause and only logs an info-level message. This could hide critical errors during CDash submission. Consider logging at warning or error level, and potentially re-raising unexpected exceptions while only catching specific expected exceptions (like PermissionError or OSError for temp directory issues).

Suggested change

except Exception as e:

logging.info(f"Temp dir '{tmpdir}' failed with error {e}")

except (OSError, PermissionError) as e:

logging.warning(f"Temp dir '{tmpdir}' failed with error {e}", exc_info=True)

Copilot · 2026-01-30T18:21:02Z

CIME/wait_for_tests.py

+                        return
+
+        except Exception as e:
+            logging.info(f"Temp dir '{tmpdir}' failed with error {e}")


The variable tmpdir is used in the exception handler but is only defined within the with statement context. If an exception occurs before tmpdir is assigned or during the context manager setup, this will raise a NameError. Consider defining tmpdir = tmproot before the try block or using tmproot in the error message instead.

Suggested change

logging.info(f"Temp dir '{tmpdir}' failed with error {e}")

logging.info(f"Temp dir operation failed with error {e}")

jgfouca added 5 commits January 29, 2026 15:09

wait_for_tests: Refactor and restore commit/time upload

3f0f872

Fix

70ddd2d

Fix

4a4e1d0

Fixes

93bea97

black

5214b17

jgfouca requested review from jasonb5 and rljacob January 29, 2026 22:49

jgfouca self-assigned this Jan 29, 2026

Copilot AI review requested due to automatic review settings January 29, 2026 22:49

jgfouca added the Responsibility: E3SM Responsibility to manage and accomplish this issue is through E3SM label Jan 29, 2026

Copilot started reviewing on behalf of jgfouca January 29, 2026 22:49 View session

Copilot AI reviewed Jan 29, 2026

View reviewed changes

rljacob approved these changes Jan 30, 2026

View reviewed changes

jgfouca added 3 commits January 30, 2026 09:15

Fixes from copilot

e43b9ac

Add support for picking a tmproot. Add support for testing

7d7b7b7

black

303fe74

jgfouca requested a review from Copilot January 30, 2026 18:13

Copilot started reviewing on behalf of jgfouca January 30, 2026 18:14 View session

Copilot AI reviewed Jan 30, 2026

View reviewed changes

jgfouca added 2 commits January 30, 2026 11:51

Fix copilot review items

8831bb4

black

b8f7125

jgfouca merged commit 66dd00b into master Jan 30, 2026
9 checks passed

jgfouca deleted the jgfouca/wait_for_tests_refactor branch January 30, 2026 22:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wait_for_tests: cdash Refactor and restore commit/time upload #4922

wait_for_tests: cdash Refactor and restore commit/time upload #4922

jgfouca commented Jan 29, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 29, 2026

Uh oh!

Copilot AI Jan 29, 2026

Uh oh!

Copilot AI Jan 29, 2026

Uh oh!

Copilot AI Jan 29, 2026

Uh oh!

Copilot AI Jan 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 30, 2026

Uh oh!

Copilot AI Jan 30, 2026

Uh oh!

Copilot AI Jan 30, 2026

Uh oh!

Copilot AI Jan 30, 2026

Uh oh!

Copilot AI Jan 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	with tempfile.TemporaryDirectory(prefix=prefix) as tmpdir:
	with tempfile.TemporaryDirectory(dir=prefix) as tmpdir:

-                        f"Commit {git_commit}\nTotal testing time {time_info} seconds\n"
+                        f"Commit {git_commit}\n"
+                        f"Total testing time: "
+                        f"{time_info if isinstance(time_info, str) else f'{time_info} seconds'}\n"

	logging.info(f"Prexix '{prefix}' failed with error {e}")
	logging.info(f"Prefix '{prefix}' failed with error {e}")

		notes_fd.write(
		f"Commit {git_commit}\nTotal testing time {time_info} seconds\n"

-                    notes_fd.write(
-                        f"Commit {git_commit}\nTotal testing time {time_info} seconds\n"
+                    if time_info == "unknown":
+                        total_time_line = "Total testing time: unknown"
+                    else:
+                        total_time_line = f"Total testing time {time_info} seconds"
+                    notes_fd.write(
+                        f"Commit {git_commit}\n{total_time_line}\n"

		except Exception as e:
		logging.info(f"Temp dir '{tmpdir}' failed with error {e}")

	logging.info(f"Temp dir '{tmpdir}' failed with error {e}")
	logging.info(f"Temp dir operation failed with error {e}")

wait_for_tests: cdash Refactor and restore commit/time upload #4922

wait_for_tests: cdash Refactor and restore commit/time upload #4922

Conversation

jgfouca commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jgfouca commented Jan 29, 2026 •

edited

Loading