[stable-2.12] ansible-test - Fix timeout handling (#80764). (#80768)

(cherry picked from commit 4c6aa70662e6f2804686a32bea611a8aa870b363)

7 files changed, 83 insertions, 36 deletions

diff --git a/changelogs/fragments/ansible-test-timeout-fix.yml b/changelogs/fragments/ansible-test-timeout-fix.yml
new file mode 100644
index 0000000000..046d5b46d3
--- /dev/null
+++ b/changelogs/fragments/ansible-test-timeout-fix.yml
@@ -0,0 +1,5 @@
+bugfixes:
+  - ansible-test - Fix various cases where the test timeout could expire without terminating the tests.
+minor_changes:
+  - ansible-test - Refactored ``env`` command logic and timeout handling.
+  - ansible-test - Allow float values for the ``--timeout`` option to the ``env`` command. This simplifies testing.
diff --git a/test/lib/ansible_test/_internal/__init__.py b/test/lib/ansible_test/_internal/__init__.py
index 18e776ad74..fb3c7b396a 100644
--- a/test/lib/ansible_test/_internal/__init__.py
+++ b/test/lib/ansible_test/_internal/__init__.py
@@ -18,6 +18,7 @@ from .constants import (
 from .util import (
     ApplicationError,
     HostConnectionError,
+    TimeoutExpiredError,
     display,
 )
 
@@ -103,6 +104,9 @@ def main():
     except ApplicationError as ex:
         display.fatal(u'%s' % ex)
         sys.exit(1)
+    except TimeoutExpiredError as ex:
+        display.fatal('%s' % ex)
+        sys.exit(1)
     except KeyboardInterrupt:
         sys.exit(2)
     except BrokenPipeError:
diff --git a/test/lib/ansible_test/_internal/cli/commands/env.py b/test/lib/ansible_test/_internal/cli/commands/env.py
index 53437a1f96..2a957a1b96 100644
--- a/test/lib/ansible_test/_internal/cli/commands/env.py
+++ b/test/lib/ansible_test/_internal/cli/commands/env.py
@@ -55,7 +55,7 @@ def do_env(
 
     env.add_argument(
         '--timeout',
-        type=int,
+        type=float,
         metavar='MINUTES',
         help='timeout for future ansible-test commands (0 clears)',
     )
diff --git a/test/lib/ansible_test/_internal/commands/env/__init__.py b/test/lib/ansible_test/_internal/commands/env/__init__.py
index 967f091faa..90078553a9 100644
--- a/test/lib/ansible_test/_internal/commands/env/__init__.py
+++ b/test/lib/ansible_test/_internal/commands/env/__init__.py
@@ -42,16 +42,20 @@ from ...ci import (
     get_ci_provider,
 )
 
+from ...timeout import (
+    TimeoutDetail,
+)
+
 
 class EnvConfig(CommonConfig):
     """Configuration for the `env` command."""
     def __init__(self, args):  # type: (t.Any) -> None
         super().__init__(args, 'env')
 
-        self.show = args.show
-        self.dump = args.dump
-        self.timeout = args.timeout
-        self.list_files = args.list_files
+        self.show: bool = args.show
+        self.dump: bool = args.dump
+        self.timeout: int | float | None = args.timeout
+        self.list_files: bool = args.list_files
 
         if not self.show and not self.dump and self.timeout is None and not self.list_files:
             # default to --show if no options were given
@@ -124,25 +128,18 @@ def set_timeout(args):  # type: (EnvConfig) -> None
     if args.timeout is None:
         return
 
-    if args.timeout:
-        deadline = (datetime.datetime.utcnow() + datetime.timedelta(minutes=args.timeout)).strftime('%Y-%m-%dT%H:%M:%SZ')
+    timeout = TimeoutDetail.create(args.timeout)
 
-        display.info('Setting a %d minute test timeout which will end at: %s' % (args.timeout, deadline), verbosity=1)
+    if timeout:
+        display.info(f'Setting a {timeout.duration} minute test timeout which will end at: {timeout.deadline}', verbosity=1)
     else:
-        deadline = None
-
         display.info('Clearing existing test timeout.', verbosity=1)
 
     if args.explain:
         return
 
-    if deadline:
-        data = dict(
-            duration=args.timeout,
-            deadline=deadline,
-        )
-
-        write_json_file(TIMEOUT_PATH, data)
+    if timeout:
+        write_json_file(TIMEOUT_PATH, timeout.to_dict())
     elif os.path.exists(TIMEOUT_PATH):
         os.remove(TIMEOUT_PATH)
 
diff --git a/test/lib/ansible_test/_internal/test.py b/test/lib/ansible_test/_internal/test.py
index 2eac09c601..437e2201e5 100644
--- a/test/lib/ansible_test/_internal/test.py
+++ b/test/lib/ansible_test/_internal/test.py
@@ -127,7 +127,7 @@ class TestResult:
 
 class TestTimeout(TestResult):
     """Test timeout."""
-    def __init__(self, timeout_duration):  # type: (int) -> None
+    def __init__(self, timeout_duration: int | float) -> None:
         super().__init__(command='timeout', test='')
 
         self.timeout_duration = timeout_duration
diff --git a/test/lib/ansible_test/_internal/timeout.py b/test/lib/ansible_test/_internal/timeout.py
index c255f5ce9f..341cb8fd4f 100644
--- a/test/lib/ansible_test/_internal/timeout.py
+++ b/test/lib/ansible_test/_internal/timeout.py
@@ -1,6 +1,7 @@
 """Timeout management for tests."""
 from __future__ import annotations
 
+import dataclasses
 import datetime
 import functools
 import os
@@ -19,7 +20,7 @@ from .config import (
 
 from .util import (
     display,
-    ApplicationError,
+    TimeoutExpiredError,
 )
 
 from .thread import (
@@ -35,15 +36,56 @@ from .test import (
 )
 
 
-def get_timeout():  # type: () -> t.Optional[t.Dict[str, t.Any]]
-    """Return details about the currently set timeout, if any, otherwise return None."""
-    if not os.path.exists(TIMEOUT_PATH):
-        return None
+@dataclasses.dataclass(frozen=True)
+class TimeoutDetail:
+    """Details required to enforce a timeout on test execution."""
+
+    _DEADLINE_FORMAT = '%Y-%m-%dT%H:%M:%SZ'  # format used to maintain backwards compatibility with previous versions of ansible-test
+
+    deadline: datetime.datetime
+    duration: int | float  # minutes
+
+    @property
+    def remaining(self) -> datetime.timedelta:
+        """The amount of time remaining before the timeout occurs. If the timeout has passed, this will be a negative duration."""
+        return self.deadline - datetime.datetime.now(tz=datetime.timezone.utc).replace(microsecond=0)
+
+    def to_dict(self) -> dict[str, t.Any]:
+        """Return timeout details as a dictionary suitable for JSON serialization."""
+        return dict(
+            deadline=self.deadline.strftime(self._DEADLINE_FORMAT),
+            duration=self.duration,
+        )
 
-    data = read_json_file(TIMEOUT_PATH)
-    data['deadline'] = datetime.datetime.strptime(data['deadline'], '%Y-%m-%dT%H:%M:%SZ')
+    @staticmethod
+    def from_dict(value: dict[str, t.Any]) -> TimeoutDetail:
+        """Return a TimeoutDetail instance using the value previously returned by to_dict."""
+        return TimeoutDetail(
+            deadline=datetime.datetime.strptime(value['deadline'], TimeoutDetail._DEADLINE_FORMAT).replace(tzinfo=datetime.timezone.utc),
+            duration=value['duration'],
+        )
 
-    return data
+    @staticmethod
+    def create(duration: int | float) -> TimeoutDetail | None:
+        """Return a new TimeoutDetail instance for the specified duration (in minutes), or None if the duration is zero."""
+        if not duration:
+            return None
+
+        if duration == int(duration):
+            duration = int(duration)
+
+        return TimeoutDetail(
+            deadline=datetime.datetime.now(datetime.timezone.utc).replace(microsecond=0) + datetime.timedelta(seconds=int(duration * 60)),
+            duration=duration,
+        )
+
+
+def get_timeout() -> TimeoutDetail | None:
+    """Return details about the currently set timeout, if any, otherwise return None."""
+    try:
+        return TimeoutDetail.from_dict(read_json_file(TIMEOUT_PATH))
+    except FileNotFoundError:
+        return None
 
 
 def configure_timeout(args):  # type: (CommonConfig) -> None
@@ -59,27 +101,22 @@ def configure_test_timeout(args):  # type: (TestConfig) -> None
     if not timeout:
         return
 
-    timeout_start = datetime.datetime.utcnow()
-    timeout_duration = timeout['duration']
-    timeout_deadline = timeout['deadline']
-    timeout_remaining = timeout_deadline - timeout_start
+    timeout_remaining = timeout.remaining
 
-    test_timeout = TestTimeout(timeout_duration)
+    test_timeout = TestTimeout(timeout.duration)
 
     if timeout_remaining <= datetime.timedelta():
         test_timeout.write(args)
 
-        raise ApplicationError('The %d minute test timeout expired %s ago at %s.' % (
-            timeout_duration, timeout_remaining * -1, timeout_deadline))
+        raise TimeoutExpiredError(f'The {timeout.duration} minute test timeout expired {timeout_remaining * -1} ago at {timeout.deadline}.')
 
-    display.info('The %d minute test timeout expires in %s at %s.' % (
-        timeout_duration, timeout_remaining, timeout_deadline), verbosity=1)
+    display.info(f'The {timeout.duration} minute test timeout expires in {timeout_remaining} at {timeout.deadline}.', verbosity=1)
 
     def timeout_handler(_dummy1, _dummy2):
         """Runs when SIGUSR1 is received."""
         test_timeout.write(args)
 
-        raise ApplicationError('Tests aborted after exceeding the %d minute time limit.' % timeout_duration)
+        raise TimeoutExpiredError(f'Tests aborted after exceeding the {timeout.duration} minute time limit.')
 
     def timeout_waiter(timeout_seconds):  # type: (int) -> None
         """Background thread which will kill the current process if the timeout elapses."""
diff --git a/test/lib/ansible_test/_internal/util.py b/test/lib/ansible_test/_internal/util.py
index 0c90901e87..1771e8df8c 100644
--- a/test/lib/ansible_test/_internal/util.py
+++ b/test/lib/ansible_test/_internal/util.py
@@ -905,6 +905,10 @@ class ApplicationWarning(Exception):
     """General application warning which interrupts normal program flow."""
 
 
+class TimeoutExpiredError(SystemExit):
+    """Error raised when the test timeout has been reached or exceeded."""
+
+
 class SubprocessError(ApplicationError):
     """Error resulting from failed subprocess execution."""
     def __init__(