From 1042c9099ded9e280b0ea64ef69df8781d326f99 Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Mon, 24 Feb 2025 16:27:35 +0100 Subject: [PATCH] feat: test for solver timeout behavior (#1865) * feat: test solver timeout behavior * Self-review * Fix trailing whitespace --- src/alire/alire-install.adb | 2 +- src/alire/alire-roots-editable.adb | 11 +++- src/alire/alire-roots.adb | 2 +- src/alire/alire-settings-builtins.ads | 18 ++++++ src/alire/alire-settings.ads | 2 + src/alire/alire-solver-predefined_options.ads | 2 +- src/alire/alire-solver.adb | 47 +++++++++++---- src/alire/alire-solver.ads | 22 ++++--- src/alr/alr-commands-get.adb | 2 +- src/alr/alr-commands-search.adb | 37 ++++++++---- src/alr/alr-commands-show.adb | 2 +- testsuite/drivers/alr.py | 11 ++-- testsuite/tests/solver/timeout/test.py | 59 +++++++++++++++++++ testsuite/tests/solver/timeout/test.yaml | 6 ++ 14 files changed, 186 insertions(+), 37 deletions(-) create mode 100644 testsuite/tests/solver/timeout/test.py create mode 100644 testsuite/tests/solver/timeout/test.yaml diff --git a/src/alire/alire-install.adb b/src/alire/alire-install.adb index 133fcda6..b424a560 100644 --- a/src/alire/alire-install.adb +++ b/src/alire/alire-install.adb @@ -188,7 +188,7 @@ package body Alire.Install is -- Look for a regular solution to a dependency as fallback if we -- didn't find any binary solution. procedure Compute_Regular (Dep : Dependencies.Dependency) is - Sol : constant Solutions.Solution := Solver.Resolve (Dep); + Sol : constant Solutions.Solution := Solver.Resolve (Dep).Solution; begin if Sol.Is_Complete then Result.Insert (Dep.Crate, Sol); diff --git a/src/alire/alire-roots-editable.adb b/src/alire/alire-roots-editable.adb index 1cd78e6e..85917a43 100644 --- a/src/alire/alire-roots-editable.adb +++ b/src/alire/alire-roots-editable.adb @@ -6,6 +6,7 @@ with Alire.Directories; with Alire.Index; with Alire.Manifest; with Alire.Roots.Optional; +with Alire.Solver.Predefined_Options; with Alire.User_Pins; with Alire.Utils.User_Input; with Alire.VCSs.Git; @@ -106,7 +107,15 @@ package body Alire.Roots.Editable is .Dependencies (This.Edit.Environment) and Dep, Props => This.Edit.Environment, - Pins => This.Edit.Pins); + Pins => This.Edit.Pins, + Options => Solver.Predefined_Options.Best_Effort) + .Solution; + -- Here we are internally looking for a solution, but we do + -- not want to nag the user in case of trouble, as it would be + -- confusing (this is not the main search for a solution with all + -- the new dependencies, which will do ask). So we use Best_Effort + -- to come with a solution quickly (usual case) or abandon + -- silently. begin if Sol.State (Dep.Crate).Has_Release then return diff --git a/src/alire/alire-roots.adb b/src/alire/alire-roots.adb index 9a3a2b4c..4d6da7dc 100644 --- a/src/alire/alire-roots.adb +++ b/src/alire/alire-roots.adb @@ -1945,7 +1945,7 @@ package body Alire.Roots is (Deps => Deps, Props => This.Environment, Pins => This.Pins, - Options => Options); + Options => Options).Solution; end Compute_Update; ------------------------- diff --git a/src/alire/alire-settings-builtins.ads b/src/alire/alire-settings-builtins.ads index d468e0da..c6bc7b5e 100644 --- a/src/alire/alire-settings-builtins.ads +++ b/src/alire/alire-settings-builtins.ads @@ -147,6 +147,24 @@ package Alire.Settings.Builtins is "If true, `alr with` will replace 'any' dependencies with the" & " appropriate caret/tilde dependency."); + Solver_Timeout : constant Builtin := New_Builtin + (Key => "solver.timeout", + Kind => Stn_Int, + Def => "5", + Help => "Seconds until solver first timeout (-1 to disable)"); + + Solver_Grace_Period : constant Builtin := New_Builtin + (Key => "solver.grace_period", + Kind => Stn_Int, + Def => "10", + Help => "Extra seconds to look for solutions after timeout"); + + Solver_Never_Finish : constant Builtin := New_Builtin + (Key => "solver.never_finish", + Public => False, + Def => False, + Help => "Never progress towards a solution (for testing only)"); + -- TOOLCHAIN Toolchain_Assistant : constant Builtin := New_Builtin diff --git a/src/alire/alire-settings.ads b/src/alire/alire-settings.ads index 6ad48853..7f4bead3 100644 --- a/src/alire/alire-settings.ads +++ b/src/alire/alire-settings.ads @@ -44,6 +44,8 @@ package Alire.Settings is function Get (This : Builtin_Option) return Boolean; function Get (This : Builtin_Option) return Setting_Int; + function Get_Int (This : Builtin_Option) return Setting_Int renames Get; + procedure Set_Locally (This : Builtin_Option; Value : String); procedure Set_Globally (This : Builtin_Option; Value : String); diff --git a/src/alire/alire-solver-predefined_options.ads b/src/alire/alire-solver-predefined_options.ads index 338f5767..a4cd2409 100644 --- a/src/alire/alire-solver-predefined_options.ads +++ b/src/alire/alire-solver-predefined_options.ads @@ -10,7 +10,7 @@ package Alire.Solver.Predefined_Options is Interactive : constant Query_Options := (Stopping => Ask, others => <>); - -- Ask of timeout + -- Ask on timeout Exhaustive : constant Query_Options := (Stopping => Continue, diff --git a/src/alire/alire-solver.adb b/src/alire/alire-solver.adb index 4bb4b2a0..fa18fe29 100644 --- a/src/alire/alire-solver.adb +++ b/src/alire/alire-solver.adb @@ -267,7 +267,9 @@ package body Alire.Solver is Pins : Solution; Options : Query_Options := Default_Options) return Boolean - is (Resolve (Deps, Props, Pins, Options).Is_Complete); + is (Resolve (Deps, Props, Pins, Options).Solution.Is_Complete); + -- No need to check the Timed_Out field; if the solution is complete it has + -- to be the first one found, and it was found before timing out. ------------- -- Resolve -- @@ -276,7 +278,7 @@ package body Alire.Solver is function Resolve (Dep : Dependencies.Dependency; Options : Query_Options := Default_Options) - return Solution + return Result is (Resolve (Deps => Conditional.New_Dependency (Dep), Props => Platforms.Current.Properties, Pins => Solutions.Empty_Valid_Solution, @@ -290,7 +292,7 @@ package body Alire.Solver is Props : Properties.Vector; Pins : Solution; Options : Query_Options := Default_Options) - return Solution + return Result is Tmp_Pool : System.Pool_Local.Unbounded_Reclaim_Pool; -- We use a local pool for easy reduction of copying of large states, @@ -326,7 +328,15 @@ package body Alire.Solver is -- To avoid reporting progress too often, as this will have some impact -- on time spent searching. + Stall : constant Boolean := Settings.Builtins.Solver_Never_Finish.Get; + -- If Stalling, we will never end the search, but timeouts will trigger. + -- We use this option for testing. + Timeout : Duration := Options.Timeout; + -- We grow the timeout if asked to look during extra time + + Timed_Out : Boolean := False; + -- Set to True if timed out before completing use Alire.Conditional.For_Dependencies; @@ -941,7 +951,14 @@ package body Alire.Solver is is when Left => return Left ("id"); when Right => return Right ("id"); - when others => raise Program_Error with "impossible"; + when others => + if Stall then + -- When stalling we are not removing states, so eventually + -- we compare an state against itself. + return Left ("id"); + else + raise Program_Error with "impossible"; + end if; end case; end Is_Better; @@ -2089,11 +2106,14 @@ package body Alire.Solver is end Ask_User_To_Continue; begin - if Timer.Elapsed < Timeout then + if Timeout < 0.0 or else Timer.Elapsed < Timeout then + -- Timeout < 0.0 means no timeout at all return False; end if; - return Ask_User_To_Continue = Stop; + return Stopped : constant Boolean := Ask_User_To_Continue = Stop do + Timed_Out := Stopped; + end return; end Search_Timeout; ------------- @@ -2137,7 +2157,11 @@ package body Alire.Solver is begin Explored := Explored + 1; - States.Delete_First; + if not Stall then + -- When stalling we don't want to exhaust states, so we do + -- not remove them. + States.Delete_First; + end if; -- TODO: we could free memory here if we observe large memory -- use, although the point of using an arena is to avoid manual -- memory tinkering. @@ -2153,7 +2177,8 @@ package body Alire.Solver is Top_Ten; - exit when Solutions.Found_Best; + -- Never exit search when stalling + exit when not Stall and then Solutions.Found_Best; exit when Search_Timeout; end loop; end Explore; @@ -2255,7 +2280,8 @@ package body Alire.Solver is if Full_Dependencies.Is_Empty then Trace.Debug ("SOLVER: returning trivial solution for empty dependencies"); - return Trivial_Solution; + return (Solution => Trivial_Solution, + Timed_Out => False); end if; -- Preprocess direct dependencies to identify any impossible ones @@ -2290,7 +2316,8 @@ package body Alire.Solver is -- stopping criteria, or have fully explored the solution space. In any -- case, we will have a best solution. - return Solution_With_Extras; + return (Solution => Solution_With_Extras, + Timed_Out => Timed_Out); end Resolve; diff --git a/src/alire/alire-solver.ads b/src/alire/alire-solver.ads index 5091681c..31059391 100644 --- a/src/alire/alire-solver.ads +++ b/src/alire/alire-solver.ads @@ -3,6 +3,7 @@ with Alire.Index; with Alire.Origins; with Alire.Properties; with Alire.Releases; +with Alire.Settings.Builtins; with Alire.Solutions; with Alire.Types; with Alire.User_Pins.Maps; @@ -105,10 +106,12 @@ package Alire.Solver is Detecting : Detection_Policies := Detect; Hinting : Hinting_Policies := Hint; - Timeout : Duration := 5.0; + Timeout : Duration + := Duration (Settings.Builtins.Solver_Timeout.Get_Int); -- Time until reporting problems finding a complete solution - Timeout_More : Duration := 10.0; + Timeout_More : Duration + := Duration (Settings.Builtins.Solver_Grace_Period.Get_Int); -- Extra period if the user wants to keep looking end record; @@ -117,10 +120,15 @@ package Alire.Solver is -- See child package Predefined_Options for more. + type Result is record + Solution : Solutions.Solution; + Timed_Out : Boolean; + end record; + function Resolve (Dep : Dependencies.Dependency; Options : Query_Options := Default_Options) - return Solution; + return Result; -- For when we only know the root crate without a precise version and want -- either a complete solution or a reasonable idea of what's preventing it. -- E.g., in `alr get` and `alr install`. @@ -129,18 +137,18 @@ package Alire.Solver is Props : Properties.Vector; Pins : Solution; Options : Query_Options := Default_Options) - return Solution; + return Result; -- Exhaustively look for a solution to the given dependencies, under the -- given platform properties and lookup options. Pins can be supplied to - -- override Deps. May raise No_Solution_Error when not using Exhaustive - -- options. + -- override Deps. A solution is always returned; in the worst case it is + -- a trivially unsolved one (all dependencies skipped). function Is_Resolvable (Deps : Types.Abstract_Dependencies; Props : Properties.Vector; Pins : Solution; Options : Query_Options := Default_Options) return Boolean; - -- Simplified call to Resolve, discarding result + -- Simplified call to Resolve, discarding result. False if timed out. private diff --git a/src/alr/alr-commands-get.adb b/src/alr/alr-commands-get.adb index f1f41fae..284bb074 100644 --- a/src/alr/alr-commands-get.adb +++ b/src/alr/alr-commands-get.adb @@ -98,7 +98,7 @@ package body Alr.Commands.Get is Solution := Query.Resolve (Rel.Dependencies (Platform.Properties), Platform.Properties, - Alire.Solutions.Empty_Valid_Solution); + Alire.Solutions.Empty_Valid_Solution).Solution; Diff := Alire.Solutions.Empty_Valid_Solution.Changes (Solution); if not Solution.Is_Complete then diff --git a/src/alr/alr-commands-search.adb b/src/alr/alr-commands-search.adb index becb67df..5b161d03 100644 --- a/src/alr/alr-commands-search.adb +++ b/src/alr/alr-commands-search.adb @@ -31,6 +31,7 @@ package body Alr.Commands.Search is Flag_Unav : constant String := TTY.Error ("U"); Flag_Unsolv : constant String := TTY.Error ("X"); Flag_External : constant String := TTY.Warn ("E"); + Flag_Unk_Solv : constant String := TTY.Dim ("?"); ------------------- -- Print_Release -- @@ -41,6 +42,29 @@ package body Alr.Commands.Search is is package Solver renames Alire.Solver; + ---------------- + -- Resolvable -- + ---------------- + + function Resolvable return String is + Result : constant Solver.Result := + Solver.Resolve + (R.Dependencies (Platform.Properties), + Platform.Properties, + Alire.Solutions.Empty_Valid_Solution, + Options => (Age => Query_Policy, + Stopping => Solver.Stop, + others => <>)); + begin + if Result.Solution.Is_Complete then + return " "; + elsif Result.Timed_Out then + return Flag_Unk_Solv; + else + return Flag_Unsolv; + end if; + end Resolvable; + begin Trace.Debug ("Listing release: " & R.Milestone.TTY_Image); Found := Found + 1; @@ -55,16 +79,9 @@ package body Alr.Commands.Search is then (if R.Dependencies (Platform.Properties).Is_Empty then " " - else TTY.Dim ("?")) - elsif Solver.Is_Resolvable - (R.Dependencies (Platform.Properties), - Platform.Properties, - Alire.Solutions.Empty_Valid_Solution, - Options => (Age => Query_Policy, - Stopping => Solver.Stop, - others => <>)) - then " " - else Flag_Unsolv))); + else Flag_Unk_Solv) + else + Resolvable))); Tab.Append (TTY.Version (Semantic_Versioning.Image (R.Version))); Tab.Append (TTY.Description (R.Description)); Tab.Append (R.Notes); diff --git a/src/alr/alr-commands-show.adb b/src/alr/alr-commands-show.adb index af805a32..0d255846 100644 --- a/src/alr/alr-commands-show.adb +++ b/src/alr/alr-commands-show.adb @@ -110,7 +110,7 @@ package body Alr.Commands.Show is Platform.Properties, Alire.Solutions.Empty_Valid_Solution, Options => (Age => Query_Policy, - others => <>))); + others => <>)).Solution); begin if Cmd.Solve then Needed.Print (Rel, diff --git a/testsuite/drivers/alr.py b/testsuite/drivers/alr.py index bbe51820..9438a628 100644 --- a/testsuite/drivers/alr.py +++ b/testsuite/drivers/alr.py @@ -138,15 +138,18 @@ def run_alr_interactive(args: list[str], output: list[str], input: list[str], Run "alr" with the given arguments, feeding it the given input. No other arguments like -q or -d are added (except --no-color). + Returns the output of the command, with CRLF replaced by LF. + :param args: List of arguments to pass to "alr". :param output: List of strings expected to be output by the subprocess. :param input: List of strings to feed to the subprocess's standard input. - :param timeout: Timeout in seconds for the subprocess to complete. + :param timeout: Timeout in seconds for the subprocess to complete. If + exceeded, RuntimeError is raised. :param complain_on_error: If True and the subprocess exits with a non-zero status code, print information on the standard output (for debugging) - and raise a CalledProcessError (to abort the test). - Conversely if False and the process ends without error, it's presumed - an error was expected and CalledProcessError is raised too. + and raise a CalledProcessError (to abort the test). Conversely if False + and the process ends without error, it's presumed an error was expected + and CalledProcessError is raised too. """ # Check whether on Windows to fail early (revisit if pexpect is updated?) diff --git a/testsuite/tests/solver/timeout/test.py b/testsuite/tests/solver/timeout/test.py new file mode 100644 index 00000000..101f1574 --- /dev/null +++ b/testsuite/tests/solver/timeout/test.py @@ -0,0 +1,59 @@ +""" +Test solver timeout behaviors +""" + +import re +from drivers.alr import alr_settings_set, alr_with, init_local_crate, run_alr, run_alr_interactive +from drivers.asserts import assert_not_substring, assert_substring + +TELLTALE = "SOLVER: search timeout" + +# Configure solver for immediate timeouts +alr_settings_set("solver.timeout", "0") +alr_settings_set("solver.grace_period", "0") + +# Configure solver to never find a solution in order to force timeouts +alr_settings_set("solver.never_finish", "true") + +# First, check a regular timeout does happen for non-interactive execution, +# which completes as expected. +assert_substring(TELLTALE, + run_alr("-vv", "show", "--solve", "hello", quiet=False).out) + +# Check that plain `search` never asks, as no solving is happening +assert_not_substring(TELLTALE, + run_alr("-vv", "search", "hello", quiet=False).out) + +# Check that `search --solve` does not ask even in interactive mode, as it does +# a best-effort solving. If there were unexpected interactivity, we would get a +# RuntimeError here. Still, there is an internal solver timeout that we forced. +assert_substring(\ + TELLTALE, + run_alr_interactive(["-vv", "search", "hello", "--solve"], [], [])) + +# Check other commands that should have interactive timeouts. Enter a crate +# beforehand to take advantage of commands that require one. + +init_local_crate() +alr_with("hello") + +SOLVER_ASKS=re.compile(".*" + \ + re.escape("[Y] Yes [N] No [A] Always (default is Yes)") + \ + "\s*$") +WITH_ASKS=re.compile(".*" + \ + re.escape("[Y] Yes [N] No (default is No)") + \ + "\s*$") + +for cmd, output, input in\ + [(["show", "--solve", "hello"], [SOLVER_ASKS], ["n"]), # Exit on 1st timeout + (["show", "--solve", "hello"], [SOLVER_ASKS, SOLVER_ASKS], ["y", "n"]), # Exit on 2nd timeout + (["show", "--tree", "hello"], [SOLVER_ASKS], ["n"]), + (["update"], [SOLVER_ASKS], ["n"]), + (["with", "libhello"], [SOLVER_ASKS, WITH_ASKS], ["n", "n"]) + # First question is the solver asking for more time to find a solution, + # second is the user being asked to accept an incomplete solution. + ]: + run_alr_interactive(cmd, output, input) + + +print("SUCCESS") diff --git a/testsuite/tests/solver/timeout/test.yaml b/testsuite/tests/solver/timeout/test.yaml new file mode 100644 index 00000000..3780bb47 --- /dev/null +++ b/testsuite/tests/solver/timeout/test.yaml @@ -0,0 +1,6 @@ +driver: python-script +control: + - [SKIP, "skip_unix", "Test is Unix-only"] +indexes: + basic_index: + in_fixtures: true -- 2.39.5