Tests: a timing assertion measures the property, not the machine it ran on

Jint.Tests.CommonScripts/SunSpiderTests.cs

@@ -6,9 +6,25 @@ namespace Jint.Tests.CommonScripts;
 [Parallelizable(ParallelScope.All)]
 public class SunSpiderTests
 {
+    /// <summary>
+    /// What a single regular-expression match in these scripts is given.
+    /// </summary>
+    /// <remarks>
+    /// Nothing in this suite asserts anything about <c>Options.Constraints.RegexTimeout</c>; every test here
+    /// asserts that a real-world script produces the right answer. The engine's ten-second default is
+    /// therefore a wedge ceiling, and it was one sized for a machine running a single script: this fixture is
+    /// <c>[Parallelizable(ParallelScope.All)]</c>, so twenty-eight CPU-bound workloads share whatever cores
+    /// the runner has, and a Windows leg has been observed taking 7 m 19 s for the twenty-eight against ~20 s
+    /// unloaded — with <c>RegexMatchTimeoutException</c> as the only symptom (#3358). A minute cannot be
+    /// reached by a starved matcher on a pattern these scripts contain, only by a genuinely catastrophic one,
+    /// and a catastrophic one reported after a minute is still reported. It stays finite deliberately:
+    /// <c>Regex.InfiniteMatchTimeout</c> would turn that failure into a hung run.
+    /// </remarks>
+    private static readonly TimeSpan RegexWedgeCeiling = TimeSpan.FromMinutes(1);
+
     private static void RunTest(string source)
     {
-        var engine = new Engine()
+        var engine = new Engine(options => options.Constraints.RegexTimeout = RegexWedgeCeiling)
             .SetValue("log", new Action<object>(Console.WriteLine))
             .SetValue("assert", new Action<bool, string>(static (condition, message) => condition.Should().BeTrue(message)));
 

Jint.Tests.PublicInterface/ConstraintReplacementTests.cs

@@ -21,14 +21,23 @@ public class ConstraintReplacementTests
 {
     private const string LoopScript = "var n = 0; for (var i = 0; i < 200000; i++) { n += i; } n";
 
+    /// <summary>
+    /// The widened interval in the row below. It is not a duration the test is about: the discriminator is
+    /// the one-millisecond interval that must no longer be registered, which two hundred thousand iterations
+    /// pass by four orders of magnitude on any machine. What this number decides is only whether a healthy
+    /// run finishes inside it, so it is a wedge ceiling — thirty seconds was one on an idle box and not on a
+    /// runner that has been seen stalling a two-hundred-millisecond wait for a minute (#3358).
+    /// </summary>
+    private static readonly TimeSpan WidenedInterval = TimeSpan.FromMinutes(10);
+
     [Fact]
     public void TheLaterTimeoutIsTheOneEnforced()
     {
         // a widened timeout must actually take effect, which it cannot while the earlier, stricter
         // constraint is still registered alongside it
         var engine = new Engine(o => o
             .LimitExecutionTime(TimeSpan.FromMilliseconds(1))
-            .LimitExecutionTime(TimeSpan.FromSeconds(30)));
+            .LimitExecutionTime(WidenedInterval));
 
         engine.Evaluate(LoopScript).AsNumber().Should().BeGreaterThan(0);
     }

Jint.Tests.PublicInterface/HostPromiseTimeoutTests.cs

@@ -20,8 +20,33 @@ namespace Jint.Tests.PublicInterface;
 public class HostPromiseTimeoutTests
 {
     /// <summary>
-    /// A promise nobody settles, and a budget far below the ten-second default. The elapsed time is the
-    /// assertion: reading the default instead of the configured value would park for ten seconds.
+    /// <c>Options.Constraints.PromiseTimeout</c>'s own default, and so what the wrong answer costs in each
+    /// row below where the engine's configured value is not the one asserted about.
+    /// </summary>
+    private static readonly TimeSpan DefaultPromiseTimeout = TimeSpan.FromSeconds(10);
+
+    /// <summary>
+    /// Which budget the wait actually ran under, stated by the wait itself. <c>UnwrapIfPromiseCore</c>
+    /// resolves the effective timeout into one local, hands that local to the drain, and formats the very
+    /// same local into this message — so a wait that consulted the wrong budget cannot name the right one,
+    /// and this is a witness rather than a restatement of the configuration.
+    /// </summary>
+    private static void ShouldNameTheBudgetItWaitedUnder(PromiseRejectedException rejection, TimeSpan budget)
+        => rejection.Message.Should().Contain(budget.ToString());
+
+    /// <summary>
+    /// The other half of the same claim, and the half a clock is needed for: that the wait <em>ended</em> on
+    /// the budget it names rather than sitting out the longer one it should have ignored. The bound is half
+    /// of whatever the wrong answer would have spent, so it stays a midpoint between the two candidates
+    /// instead of an absolute number a loaded runner can walk past — #3358 saw a 200 ms wait measured at
+    /// 1 m 3 s against a fixed 5 s.
+    /// </summary>
+    private static void ShouldNotHaveSpentTheOtherBudget(Stopwatch elapsed, TimeSpan wrongBudget)
+        => elapsed.Elapsed.Should().BeLessThan(TimeSpan.FromTicks(wrongBudget.Ticks / 2));
+
+    /// <summary>
+    /// A promise nobody settles, and a budget far below the ten-second default. Reading the default instead
+    /// of the configured value would name ten seconds and park for ten seconds, and both are asserted.
     /// </summary>
     [Fact]
     public void TheArgumentLessUnwrapTakesTheConfiguredPromiseTimeout()
@@ -35,10 +60,8 @@ public void TheArgumentLessUnwrapTakesTheConfiguredPromiseTimeout()
         var rejection = Assert.Throws<PromiseRejectedException>(() => manual.Promise.UnwrapIfPromise());
         elapsed.Stop();
 
-        // Generous against a loaded CI machine, and still an order of magnitude below the ten seconds the
-        // hard-coded default would have cost.
-        elapsed.Elapsed.Should().BeLessThan(TimeSpan.FromSeconds(5));
-        rejection.Message.Should().Contain(configured.ToString());
+        ShouldNameTheBudgetItWaitedUnder(rejection, configured);
+        ShouldNotHaveSpentTheOtherBudget(elapsed, DefaultPromiseTimeout);
     }
 
     /// <summary>
@@ -49,7 +72,8 @@ public void TheArgumentLessUnwrapTakesTheConfiguredPromiseTimeout()
     [Fact]
     public void AnExplicitTimeoutStillWinsOverTheConfiguredOne()
     {
-        using var engine = new Engine(options => options.Constraints.PromiseTimeout = TimeSpan.FromMinutes(5));
+        var configured = TimeSpan.FromMinutes(5);
+        using var engine = new Engine(options => options.Constraints.PromiseTimeout = configured);
         var manual = engine.Tasks.RegisterPromise();
 
         var requested = TimeSpan.FromMilliseconds(200);
@@ -58,8 +82,8 @@ public void AnExplicitTimeoutStillWinsOverTheConfiguredOne()
         var rejection = Assert.Throws<PromiseRejectedException>(() => manual.Promise.UnwrapIfPromise(requested));
         elapsed.Stop();
 
-        elapsed.Elapsed.Should().BeLessThan(TimeSpan.FromSeconds(5));
-        rejection.Message.Should().Contain(requested.ToString());
+        ShouldNameTheBudgetItWaitedUnder(rejection, requested);
+        ShouldNotHaveSpentTheOtherBudget(elapsed, configured);
     }
 
     /// <summary>

Jint.Tests.PublicInterface/HostPumpWaitTests.cs

@@ -18,11 +18,20 @@ namespace Jint.Tests.PublicInterface;
 /// </remarks>
 public class HostPumpWaitTests
 {
-    private static readonly TimeSpan Ceiling = TimeSpan.FromSeconds(10);
+    /// <summary>
+    /// What the wait under test is given. Nothing should ever spend it, so its only job is to bound a wake
+    /// that never comes — which is why it is a minute rather than the ten seconds it was through #3301: the
+    /// ratio below is the assertion, and raising both together widens the margin without weakening it.
+    /// </summary>
+    private static readonly TimeSpan Ceiling = TimeSpan.FromSeconds(60);
 
-    private static readonly TimeSpan EarlyReturnMargin = TimeSpan.FromSeconds(5);
+    /// <summary>
+    /// The margin the wake has to beat, derived from the ceiling rather than written out again so the two
+    /// cannot drift. Half of it, which is what separates "woke on the post" from "ran out the ceiling".
+    /// </summary>
+    private static readonly TimeSpan EarlyReturnMargin = TimeSpan.FromTicks(Ceiling.Ticks / 2);
 
-    private static readonly TimeSpan WedgeCeiling = TimeSpan.FromMinutes(2);
+    private static readonly TimeSpan WedgeCeiling = TestBudgets.WedgeCeiling;
 
     /// <summary>
     /// The shape the wait exists for: one thread parked on this engine, another handing it work. A settled
@@ -50,11 +59,7 @@ public async Task WaitForScheduledWorkWakesOnACrossThreadPost()
         var elapsed = new Stopwatch();
 
         engine.SetValue("hostWork", manual.Promise);
-        engine.SetValue("armProducer", new Action(() =>
-        {
-            elapsed.Start();
-            producerArmed.Set();
-        }));
+        engine.SetValue("armProducer", new Action(producerArmed.Set));
         engine.SetValue("park", new Func<bool>(() => engine.Tasks.WaitForScheduledWork(Ceiling)));
 
         var producer = DedicatedThread.RunAsync(() =>
@@ -64,6 +69,12 @@ public async Task WaitForScheduledWorkWakesOnACrossThreadPost()
             // Lets the wait be reached before the settle lands, so the wake rather than the pre-check is what
             // ends it. Both are correct and both pass — see the remarks above.
             Thread.Sleep(TimeSpan.FromMilliseconds(250));
+
+            // Started here, on the producer, immediately before the post: what the margin below is about is
+            // the wake, and starting the clock back in armProducer() also charged it for this thread being
+            // scheduled and for the settle above. That is what produced the 12 s reading in #3358 on a wake
+            // that was not slow at all.
+            elapsed.Start();
             manual.Resolve(JsValue.Undefined);
         });
 

Jint.Tests.PublicInterface/HostStreamBridgeTests.cs

@@ -24,13 +24,32 @@ namespace Jint.Tests.PublicInterface;
 /// asynchronous methods complete synchronously (see <see cref="RecordingStream"/>), so the whole copy runs on
 /// the engine's own turns and a bounded <c>ProcessTasks</c> loop is enough to drive it to completion. The one
 /// test that deliberately goes off-thread, <see cref="ReadsAStreamWhoseReadsCompleteOnAnotherThread"/>, waits
-/// for the outcome through the engine's own blocking drain rather than for an interval.
+/// for the outcome through the engine's own blocking drain rather than for an interval — under the wedge
+/// ceiling <see cref="OffThreadStreamEngine"/> configures, which is a bound on a hang and never an assertion.
 /// </para>
 /// </remarks>
 public class HostStreamBridgeTests
 {
     private static Engine StreamEngine() => new(options => options.UseWebApis(WebApiFeatures.Streams));
 
+    /// <summary>
+    /// The same engine for the one test whose chunks arrive from another thread, with the promise budget the
+    /// blocking drain runs under moved off the engine's ten-second default and onto
+    /// <see cref="TestBudgets.WedgeCeiling"/>.
+    /// </summary>
+    /// <remarks>
+    /// Nothing here asserts a duration — the assertion is the text that came out of the stream — so the
+    /// budget is a wedge ceiling and widening it can hide nothing. What it removes is the thread pool from
+    /// the set of things that decide the outcome: each chunk is delivered by a pool continuation, and on a
+    /// saturated runner the whole copy has been seen failing as <c>PromiseRejectedException: Timeout of
+    /// 00:00:10 reached</c> (#3358), which is a symptom of the machine rather than of the bridge.
+    /// </remarks>
+    private static Engine OffThreadStreamEngine() => new(options =>
+    {
+        options.UseWebApis(WebApiFeatures.Streams);
+        options.Constraints.PromiseTimeout = TestBudgets.WedgeCeiling;
+    });
+
     private static byte[] Utf8(string text) => Encoding.UTF8.GetBytes(text);
 
     /// <summary>
@@ -383,7 +402,7 @@ public void ReadsAStreamWhoseReadsCompleteOnAnotherThread()
         // The cross-thread half: every chunk arrives as a generation-stamped event-loop job rather than
         // through the synchronous window. UnwrapIfPromise is the engine's own blocking drain, which wakes on
         // the work-arrived signal rather than on a poll interval.
-        var engine = StreamEngine();
+        var engine = OffThreadStreamEngine();
         engine.SetValue("input", engine.WebApi.CreateReadableStream(
             new OffThreadStream(Utf8("streamed off-thread")),
             new HostReadableStreamOptions { ChunkSize = 4 }));

Jint.Tests.PublicInterface/UntrustedCodeProfileTests.cs

@@ -327,7 +327,15 @@ Task Attempt()
     [Fact]
     public async Task OperationScopeCannotEndWhileAsyncWorkOwnsTheEngine()
     {
-        var limits = CreateLimits();
+        // Every wall-clock limit here is a wedge ceiling rather than part of the claim: what is asserted is
+        // that a scope refuses to end while an evaluation is suspended, and then ends once it resumes. The
+        // profile's own five-second timeout interval and ten-second operation deadline are budgets an
+        // embedder would size for their own work, and leaving them at those defaults made this test fail on
+        // a loaded runner as a TimeoutException from the gate below rather than as anything about scopes.
+        var limits = CreateLimits(
+            timeoutInterval: TestBudgets.WedgeCeiling,
+            promiseTimeout: TestBudgets.WedgeCeiling,
+            maxOperationDuration: TestBudgets.WedgeCeiling);
         using var cancellation = new CancellationTokenSource();
         // TaskCompletionSource<bool> rather than the non-generic one, which only exists from .NET 5 and
         // this project also compiles for net472.

Jint.Tests/Runtime/AsyncTests.cs

@@ -965,7 +965,17 @@ public Task ShouldReturnedTaskCatchWhenThrowError() => DedicatedThread.RunAsync(
     public Task ShouldTaskAwaitCurrentStack() => DedicatedThread.RunAsync(() =>
     {
         //https://github.com/sebastienros/jint/issues/514#issuecomment-1507127509
-        Engine engine = new(options => options.ExperimentalFeatures = ExperimentalFeature.TaskInterop);
+
+        // What is asserted is the order the three appends landed in, never a duration, so the promise budget
+        // here is a wedge ceiling rather than part of the claim. The delays below add up to about 1.1 s of
+        // intended work, which the engine's ten-second default was sized for on an idle machine and not on a
+        // two-core runner whose pool grows at a worker per 500 ms: sebastienros/jint#3358 saw exactly this
+        // body fail on net472 with "Timeout of 00:00:10 reached", which says nothing about awaiting a stack.
+        Engine engine = new(options =>
+        {
+            options.ExperimentalFeatures = ExperimentalFeature.TaskInterop;
+            options.Constraints.PromiseTimeout = TestBudgets.WedgeCeiling;
+        });
         AsyncTestClass asyncTestClass = new();
 
         engine.SetValue("myAsyncMethod", new Func<Task>(async () =>
@@ -1284,7 +1294,11 @@ static async ValueTask Callable()
     public Task ShouldValueTaskAwaitCurrentStack() => DedicatedThread.RunAsync(() =>
     {
         //https://github.com/sebastienros/jint/issues/514#issuecomment-1507127509
-        Engine engine = new();
+
+        // The ValueTask twin of ShouldTaskAwaitCurrentStack, and a wedge ceiling for the same reason: the
+        // assertion is the order, and a second of intended work inside a ten-second default is not a margin
+        // a two-core runner respects.
+        Engine engine = new(options => options.Constraints.PromiseTimeout = TestBudgets.WedgeCeiling);
         string log = "";
         engine.SetValue("myAsyncMethod", new Func<ValueTask>(async () =>
         {
@@ -1308,7 +1322,7 @@ public Task ShouldReturnedValueTaskOfTConvertedToPromiseInJS() => DedicatedThrea
         Engine engine = new(options =>
         {
             options.ExperimentalFeatures = ExperimentalFeature.TaskInterop;
-            options.Constraints.PromiseTimeout = TimeSpan.FromMinutes(2);
+            options.Constraints.PromiseTimeout = TestBudgets.WedgeCeiling;
         });
         engine.SetValue("asyncTestClass", new AsyncTestClass());
         var result = engine.Evaluate("asyncTestClass.ReturnDelayedValueTaskAsync().then(x=>x)");

Jint.Tests/Runtime/PromiseTests.cs

@@ -704,10 +704,17 @@ public async Task UnwrapIfPromiseAsync_WithIOBoundTask_DoesNotBlockCallerThread(
         var engine = new Engine();
         var ioStarted = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
 
+        // The IO ends when this test says so, never on an interval. `await Task.Delay(100)` used to stand in
+        // for it, which made the "still pending" assertion below a race the test could lose: on a loaded
+        // runner the hundred milliseconds can be gone before this thread is scheduled again, and a completed
+        // unwrap then reads as the defect this test exists to catch. A gate the test holds cannot complete
+        // early, so "in flight" is a fact rather than a hope.
+        var releaseIO = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
+
         engine.SetValue("simulateIO", new Func<Task<int>>(async () =>
         {
             ioStarted.TrySetResult(true);
-            await Task.Delay(100);
+            await releaseIO.Task.ConfigureAwait(false);
             return 99;
         }));
 
@@ -722,6 +729,8 @@ public async Task UnwrapIfPromiseAsync_WithIOBoundTask_DoesNotBlockCallerThread(
         // The unwrap task should still be pending while IO is in flight
         unwrapTask.IsCompleted.Should().BeFalse("UnwrapIfPromiseAsync should not block; task should still be pending during IO");
 
+        releaseIO.SetResult(true);
+
         var result = await unwrapTask;
         result.AsInteger().Should().Be(99);
     }