chore: use manual TimeoutAttribute instead of CancelAfter attribute (#3126)

Author: mxschmitt

src/Playwright.Tests/Attributes/PlaywrightTestAttribute.cs

@@ -22,6 +22,7 @@
  * SOFTWARE.
  */
 
+using System.Reflection;
 using NUnit.Framework.Interfaces;
 using NUnit.Framework.Internal;
 using NUnit.Framework.Internal.Commands;
@@ -36,14 +37,24 @@ namespace Microsoft.Playwright.Tests;
 /// Enables decorating test facts with information about the corresponding test in the upstream repository.
 /// </summary>
 [AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
-public class PlaywrightTestAttribute : TestAttribute, IApplyToContext, IApplyToTest, IWrapSetUpTearDown
+public class PlaywrightTestAttribute : TestAttribute, IWrapSetUpTearDown
 {
-    private readonly CancelAfterAttribute _cancelAfterAttribute = new(TestConstants.DefaultTestTimeout);
+    private readonly int? _timeout;
 
     public PlaywrightTestAttribute()
     {
     }
 
+    public PlaywrightTestAttribute(int timeout) : this()
+    {
+        _timeout = timeout;
+    }
+
+    public PlaywrightTestAttribute(string fileName, string nameOfTest, int timeout) : this(fileName, nameOfTest)
+    {
+        _timeout = timeout;
+    }
+
     /// <summary>
     /// Creates a new instance of the attribute.
     /// </summary>
@@ -60,11 +71,6 @@ public PlaywrightTestAttribute(string fileName, string nameOfTest)
     /// </summary>
     public string FileName { get; }
 
-    /// <summary>
-    /// Returns the trimmed file name.
-    /// </summary>
-    public string TrimmedName => FileName.Substring(0, FileName.IndexOf('.'));
-
     /// <summary>
     /// The name of the test, the decorated code is based on.
     /// </summary>
@@ -75,29 +81,14 @@ public PlaywrightTestAttribute(string fileName, string nameOfTest)
     /// </summary>
     public string Describe { get; }
 
-    public void ApplyToContext(TestExecutionContext context)
-    {
-        if (context.TestCaseTimeout == 0)
-        {
-            (_cancelAfterAttribute as IApplyToContext).ApplyToContext(context);
-        }
-    }
-
-    public new void ApplyToTest(Test test)
-    {
-        base.ApplyToTest(test);
-        if (TestExecutionContext.CurrentContext.TestCaseTimeout == 0)
-        {
-            _cancelAfterAttribute.ApplyToTest(test);
-        }
-    }
     /// <summary>
     /// Wraps the current test command in a <see cref="UnobservedTaskExceptionCommand"/>.
     /// </summary>
     /// <param name="command">the test command</param>
     /// <returns>the wrapped test command</returns>
     public TestCommand Wrap(TestCommand command)
     {
+        command = new TimeoutCommand(command, _timeout ?? TestConstants.DefaultTestTimeout);
         if (Environment.GetEnvironmentVariable("CI") != null)
         {
             command = new RetryCommand(command, 3);
@@ -121,7 +112,7 @@ public override TestResult Execute(TestExecutionContext context)
                 try
                 {
                     innerCommand.Execute(context);
-                    if (context.CurrentResult.ResultState == ResultState.Success)
+                    if (context.CurrentResult.ResultState == ResultState.Success || context.CurrentResult.ResultState == ResultState.Skipped || context.CurrentResult.ResultState == ResultState.Ignored)
                     {
                         isPassed = true;
                         break;
@@ -205,4 +196,71 @@ private void UnobservedTaskException(object sender, UnobservedTaskExceptionEvent
             _unobservedTaskExceptions.Add(e.Exception);
         }
     }
+
+    public class TimeoutCommand : BeforeAndAfterTestCommand
+    {
+        private readonly int _timeout;
+
+        internal TimeoutCommand(TestCommand innerCommand, int timeout) : base(innerCommand)
+        {
+            _timeout = timeout;
+        }
+
+        public override TestResult Execute(TestExecutionContext context)
+        {
+            try
+            {
+                using (new TestExecutionContext.IsolatedContext())
+                {
+                    var testExecution = Task.Run(() => innerCommand.Execute(TestExecutionContext.CurrentContext));
+                    var timedOut = Task.WaitAny([testExecution], _timeout) == -1;
+
+                    if (timedOut)
+                    {
+                        context.CurrentResult.SetResult(
+                            ResultState.Failure,
+                            $"Test exceeded Timeout value of {_timeout}ms");
+                        // When the timeout is reached the TearDown methods are not called. This is a best-effort
+                        // attempt to call them and close the browser / http server.
+                        foreach (var tearDown in GetHackyTearDownMethods(context))
+                        {
+                            tearDown();
+                        }
+                    }
+                    else
+                    {
+                        context.CurrentResult = testExecution.GetAwaiter().GetResult();
+                    }
+                }
+            }
+            catch (Exception exception)
+            {
+                context.CurrentResult.RecordException(exception, FailureSite.Test);
+            }
+
+            return context.CurrentResult;
+        }
+
+        private Action[] GetHackyTearDownMethods(TestExecutionContext context)
+        {
+            var methods = new List<Action>();
+            foreach (var method in new string[] { "WorkerTeardown", "BrowserTearDown" })
+            {
+                var methodFun = context.CurrentTest.Method.MethodInfo.DeclaringType
+                    .GetMethod(method, BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic);
+                if (methodFun != null)
+                {
+                    methods.Add(() =>
+                    {
+                        var maybeTask = methodFun.Invoke(context.TestObject, null);
+                        if (maybeTask is Task task)
+                        {
+                            task.GetAwaiter().GetResult();
+                        }
+                    });
+                }
+            }
+            return methods.ToArray();
+        }
+    }
 }

src/Playwright.Tests/BaseTests/HttpService.cs

@@ -39,8 +39,8 @@ public static Task<HttpService> Register(WorkerAwareTest test)
             var assetDir = Path.Combine(TestUtils.FindParentDirectory("Playwright.Tests"), "assets");
             var http = new HttpService
             {
-                Server = SimpleServer.Create(8907 + workerIndex * 2, assetDir),
-                HttpsServer = SimpleServer.CreateHttps(8907 + workerIndex * 2 + 1, assetDir)
+                Server = SimpleServer.Create(8907 + workerIndex * 4, assetDir),
+                HttpsServer = SimpleServer.CreateHttps(8907 + workerIndex * 4 + 1, assetDir)
             };
             await Task.WhenAll(http.Server.StartAsync(TestContext.CurrentContext.CancellationToken), http.HttpsServer.StartAsync(TestContext.CurrentContext.CancellationToken));
             return http;

src/Playwright.Tests/BrowserTypeConnectTests.cs

@@ -451,8 +451,7 @@ public async Task ShouldRecordContextTraceWithNoDirectoryName()
         }
     }
 
-    [PlaywrightTest("browsertype-connect.spec.ts", "should upload large file")]
-    [CancelAfter(TestConstants.SlowTestTimeout)]
+    [PlaywrightTest("browsertype-connect.spec.ts", "should upload large file", TestConstants.SlowTestTimeout)]
     public async Task ShouldUploadLargeFile()
     {
         var browser = await BrowserType.ConnectAsync(_remoteServer.WSEndpoint);

src/Playwright.Tests/ClientCertficatesTests.cs

@@ -103,11 +103,6 @@ public void Setup()
         {
             Assert.Ignore("WebKit on macOS doesn't proxy localhost requests");
         }
-        if (TestConstants.IsChromium && TestConstants.IsWindows)
-        {
-            // TODO: Remove after https://github.com/microsoft/playwright/issues/17252 is fixed.
-            Assert.Ignore("Chromium on Windows doesn't proxy localhost requests");
-        }
     }
 
     [PlaywrightTest("", "")]

src/Playwright.Tests/PageSetInputFilesTests.cs

@@ -352,8 +352,7 @@ public async Task ShouldWorkForWebkitdirectory()
         Assert.True(fileChooser.IsMultiple);
     }
 
-    [PlaywrightTest("page-set-input-files.spec.ts", "should upload large file")]
-    [CancelAfter(TestConstants.SlowTestTimeout)]
+    [PlaywrightTest("page-set-input-files.spec.ts", "should upload large file", TestConstants.SlowTestTimeout)]
     public async Task ShouldUploadLargeFile()
     {
         await Page.GotoAsync(Server.Prefix + "/input/fileupload.html");

src/Playwright.Tests/PageWaitForNavigationTests.cs

@@ -306,8 +306,7 @@ public async Task ShouldWorkOnFrame()
         StringAssert.Contains("/frames/one-frame.html", Page.Url);
     }
 
-    [PlaywrightTest]
-    [CancelAfter(TestConstants.SlowTestTimeout)]
+    [PlaywrightTest(TestConstants.SlowTestTimeout)]
     public async Task ShouldHaveADefaultTimeout()
     {
         await Page.GotoAsync(Server.Prefix + "/frames/one-frame.html");

src/Playwright.Tests/Playwright.Tests.csproj

@@ -26,9 +26,9 @@
           <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
           <PrivateAssets>all</PrivateAssets>
         </PackageReference>
-        <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.11.1" />
-        <PackageReference Include="NUnit" Version="4.2.2" />
-        <PackageReference Include="NUnit3TestAdapter" Version="4.6.0" />
+        <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.13.0" />
+        <PackageReference Include="NUnit" Version="4.3.2" />
+        <PackageReference Include="NUnit3TestAdapter" Version="5.0.0" />
         <PackageReference Include="SixLabors.ImageSharp" Version="2.1.10" />
     </ItemGroup>
     <ItemGroup>