review feedback

mxschmitt · mxschmitt · commit c591630ec204 · 2025-01-30T17:21:11.000+01:00
diff --git a/src/Playwright.MSTest/BrowserService.cs b/src/Playwright.MSTest/BrowserService.cs
@@ -42,20 +42,20 @@ private BrowserService(IBrowser browser)
         Browser = browser;
     }
 
-    public static Task<BrowserService> Register(WorkerAwareTest test, IBrowserType browserType, PlaywrightConnectOptions? connectOptions)
+    public static Task<BrowserService> Register(WorkerAwareTest test, IBrowserType browserType, (string, BrowserTypeConnectOptions?)? connectOptions)
     {
         return test.RegisterService("Browser", async () => new BrowserService(await CreateBrowser(browserType, connectOptions).ConfigureAwait(false)));
     }
 
-    private static async Task<IBrowser> CreateBrowser(IBrowserType browserType, PlaywrightConnectOptions? connectOptions)
+    private static async Task<IBrowser> CreateBrowser(IBrowserType browserType, (string WSEndpoint, BrowserTypeConnectOptions? Options)? connectOptions)
     {
-        if (connectOptions != null)
+        if (connectOptions.HasValue && connectOptions.Value.WSEndpoint != null)
         {
-            var options = new BrowserTypeConnectOptions(connectOptions);
+            var options = new BrowserTypeConnectOptions(connectOptions?.Options ?? new());
             var headers = options.Headers?.ToDictionary(kvp => kvp.Key, kvp => kvp.Value) ?? [];
             headers.Add("x-playwright-launch-options", JsonSerializer.Serialize(PlaywrightSettingsProvider.LaunchOptions, new JsonSerializerOptions() { DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull }));
             options.Headers = headers;
-            return await browserType.ConnectAsync(connectOptions.WSEndpoint, options).ConfigureAwait(false);
+            return await browserType.ConnectAsync(connectOptions.Value.WSEndpoint, options).ConfigureAwait(false);
         }
 
         var legacyBrowser = await ConnectBasedOnEnv(browserType);
diff --git a/src/Playwright.MSTest/BrowserTest.cs b/src/Playwright.MSTest/BrowserTest.cs
@@ -62,8 +62,5 @@ public async Task BrowserTearDown()
         Browser = null!;
     }
 
-    public virtual Task<PlaywrightConnectOptions?> ConnectOptionsAsync()
-    {
-        return Task.FromResult<PlaywrightConnectOptions?>(null);
-    }
+    public virtual Task<(string, BrowserTypeConnectOptions?)?> ConnectOptionsAsync() => null!;
 }
diff --git a/src/Playwright.NUnit/BrowserService.cs b/src/Playwright.NUnit/BrowserService.cs
@@ -42,20 +42,20 @@ private BrowserService(IBrowser browser)
         Browser = browser;
     }
 
-    public static Task<BrowserService> Register(WorkerAwareTest test, IBrowserType browserType, PlaywrightConnectOptions? connectOptions)
+    public static Task<BrowserService> Register(WorkerAwareTest test, IBrowserType browserType, (string, BrowserTypeConnectOptions?)? connectOptions)
     {
         return test.RegisterService("Browser", async () => new BrowserService(await CreateBrowser(browserType, connectOptions).ConfigureAwait(false)));
     }
 
-    private static async Task<IBrowser> CreateBrowser(IBrowserType browserType, PlaywrightConnectOptions? connectOptions)
+    private static async Task<IBrowser> CreateBrowser(IBrowserType browserType, (string WSEndpoint, BrowserTypeConnectOptions? Options)? connectOptions)
     {
-        if (connectOptions != null)
+        if (connectOptions.HasValue && connectOptions.Value.WSEndpoint != null)
         {
-            var options = new BrowserTypeConnectOptions(connectOptions);
+            var options = new BrowserTypeConnectOptions(connectOptions?.Options ?? new());
             var headers = options.Headers?.ToDictionary(kvp => kvp.Key, kvp => kvp.Value) ?? [];
             headers.Add("x-playwright-launch-options", JsonSerializer.Serialize(PlaywrightSettingsProvider.LaunchOptions, new JsonSerializerOptions() { DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull }));
             options.Headers = headers;
-            return await browserType.ConnectAsync(connectOptions.WSEndpoint, options).ConfigureAwait(false);
+            return await browserType.ConnectAsync(connectOptions.Value.WSEndpoint, options).ConfigureAwait(false);
         }
 
         var legacyBrowser = await ConnectBasedOnEnv(browserType);
diff --git a/src/Playwright.NUnit/BrowserTest.cs b/src/Playwright.NUnit/BrowserTest.cs
@@ -61,8 +61,5 @@ public async Task BrowserTearDown()
         Browser = null!;
     }
 
-    public virtual Task<PlaywrightConnectOptions?> ConnectOptionsAsync()
-    {
-        return Task.FromResult<PlaywrightConnectOptions?>(null);
-    }
+    public virtual Task<(string, BrowserTypeConnectOptions?)?> ConnectOptionsAsync() => null!;
 }
diff --git a/src/Playwright.TestAdapter/PlaywrightConnectOptions.cs b/src/Playwright.TestAdapter/PlaywrightConnectOptions.cs
diff --git a/src/Playwright.TestingHarnessTest/tests/baseTest.ts b/src/Playwright.TestingHarnessTest/tests/baseTest.ts
@@ -38,7 +38,7 @@ export const test = base.extend<{
     for (const server of servers)
       await server.close();
   },
-  runTest: async ({ }, use, testInfo) => {
+  runTest: async ({ testMode }, use, testInfo) => {
     const testResults: RunResult[] = [];
     await use(async (files, command, env) => {
       const testDir = testInfo.outputPath();
diff --git a/src/Playwright.TestingHarnessTest/tests/mstest/basic.spec.ts b/src/Playwright.TestingHarnessTest/tests/mstest/basic.spec.ts
@@ -496,6 +496,7 @@ test.describe('ConnectOptions', () => {
   const ExampleTestWithConnectOptions = `
     using System;
     using System.Threading.Tasks;
+    using Microsoft.Playwright;
     using Microsoft.Playwright.MSTest;
     using Microsoft.VisualStudio.TestTools.UnitTesting;
 
@@ -509,11 +510,9 @@ test.describe('ConnectOptions', () => {
         {
             await Page.GotoAsync("about:blank");
         }
-        public override PlaywrightConnectOptions ConnectOptions()
+        public override async Task<(string, BrowserTypeConnectOptions)?> ConnectOptionsAsync()
         {
-            return new() {
-                WSEndpoint = "http://127.0.0.1:1234",
-            };
+            return ("http://127.0.0.1:1234", null);
         }
   }`;
 
diff --git a/src/Playwright.TestingHarnessTest/tests/nunit/basic.spec.ts b/src/Playwright.TestingHarnessTest/tests/nunit/basic.spec.ts
@@ -492,6 +492,7 @@ test.describe('ConnectOptions', () => {
   const ExampleTestWithConnectOptions = `
     using System;
     using System.Threading.Tasks;
+    using Microsoft.Playwright;
     using Microsoft.Playwright.NUnit;
     using NUnit.Framework;
 
@@ -505,11 +506,9 @@ test.describe('ConnectOptions', () => {
             await Page.GotoAsync("about:blank");
         }
 
-        public override PlaywrightConnectOptions ConnectOptions()
+        public override async Task<(string, BrowserTypeConnectOptions)?> ConnectOptionsAsync()
         {
-            return new() {
-                WSEndpoint = "http://127.0.0.1:1234",
-            };
+            return ("http://127.0.0.1:1234", null);
         }
   }`;
 
diff --git a/src/Playwright.TestingHarnessTest/tests/xunit/basic.spec.ts b/src/Playwright.TestingHarnessTest/tests/xunit/basic.spec.ts
@@ -542,21 +542,22 @@ test.describe('ConnectOptions', () => {
   const ExampleTestWithConnectOptions = `
     using System;
     using System.Threading.Tasks;
+    using Microsoft.Playwright;
     using Microsoft.Playwright.Xunit;
     using Xunit;
+
     namespace Playwright.TestingHarnessTest.Xunit;
+
     public class <class-name> : PageTest
     {
         [Fact]
         public async Task Test()
         {
             await Page.GotoAsync("about:blank");
         }
-        public override PlaywrightConnectOptions ConnectOptions()
+        public override async Task<(string, BrowserTypeConnectOptions)?> ConnectOptionsAsync()
         {
-            return new() {
-                WSEndpoint = "http://127.0.0.1:1234",
-            };
+            return ("http://127.0.0.1:1234", null);
         }
   }`;
 
diff --git a/src/Playwright.Xunit/BrowserService.cs b/src/Playwright.Xunit/BrowserService.cs
@@ -42,20 +42,20 @@ private BrowserService(IBrowser browser)
         Browser = browser;
     }
 
-    public static Task<BrowserService> Register(WorkerAwareTest test, IBrowserType browserType, PlaywrightConnectOptions? connectOptions)
+    public static Task<BrowserService> Register(WorkerAwareTest test, IBrowserType browserType, (string, BrowserTypeConnectOptions?)? connectOptions)
     {
         return test.RegisterService("Browser", async () => new BrowserService(await CreateBrowser(browserType, connectOptions).ConfigureAwait(false)));
     }
 
-    private static async Task<IBrowser> CreateBrowser(IBrowserType browserType, PlaywrightConnectOptions? connectOptions)
+    private static async Task<IBrowser> CreateBrowser(IBrowserType browserType, (string WSEndpoint, BrowserTypeConnectOptions? Options)? connectOptions)
     {
-        if (connectOptions != null)
+        if (connectOptions.HasValue && connectOptions.Value.WSEndpoint != null)
         {
-            var options = new BrowserTypeConnectOptions(connectOptions);
+            var options = new BrowserTypeConnectOptions(connectOptions?.Options ?? new());
             var headers = options.Headers?.ToDictionary(kvp => kvp.Key, kvp => kvp.Value) ?? [];
             headers.Add("x-playwright-launch-options", JsonSerializer.Serialize(PlaywrightSettingsProvider.LaunchOptions, new JsonSerializerOptions() { DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull }));
             options.Headers = headers;
-            return await browserType.ConnectAsync(connectOptions.WSEndpoint, options).ConfigureAwait(false);
+            return await browserType.ConnectAsync(connectOptions.Value.WSEndpoint, options).ConfigureAwait(false);
         }
 
         var legacyBrowser = await ConnectBasedOnEnv(browserType);
diff --git a/src/Playwright.Xunit/BrowserTest.cs b/src/Playwright.Xunit/BrowserTest.cs
@@ -60,8 +60,5 @@ public override async Task DisposeAsync()
         await base.DisposeAsync().ConfigureAwait(false);
     }
 
-    public virtual Task<PlaywrightConnectOptions?> ConnectOptionsAsync()
-    {
-        return Task.FromResult<PlaywrightConnectOptions?>(null);
-    }
+    public virtual Task<(string, BrowserTypeConnectOptions?)?> ConnectOptionsAsync() => null!;
 }

Original file line number	Diff line number	Diff line change
`@@ -42,20 +42,20 @@ private BrowserService(IBrowser browser)`
`42`	`42`	`Browser = browser;`
`43`	`43`	`}`
`44`	`44`
`45`		`- public static Task<BrowserService> Register(WorkerAwareTest test, IBrowserType browserType, PlaywrightConnectOptions? connectOptions)`
	`45`	`+ public static Task<BrowserService> Register(WorkerAwareTest test, IBrowserType browserType, (string, BrowserTypeConnectOptions?)? connectOptions)`
`46`	`46`	`{`
`47`	`47`	`return test.RegisterService("Browser", async () => new BrowserService(await CreateBrowser(browserType, connectOptions).ConfigureAwait(false)));`
`48`	`48`	`}`
`49`	`49`
`50`		`- private static async Task<IBrowser> CreateBrowser(IBrowserType browserType, PlaywrightConnectOptions? connectOptions)`
	`50`	`+ private static async Task<IBrowser> CreateBrowser(IBrowserType browserType, (string WSEndpoint, BrowserTypeConnectOptions? Options)? connectOptions)`
`51`	`51`	`{`
`52`		`- if (connectOptions != null)`
	`52`	`+ if (connectOptions.HasValue && connectOptions.Value.WSEndpoint != null)`
`53`	`53`	`{`
`54`		`- var options = new BrowserTypeConnectOptions(connectOptions);`
	`54`	`+ var options = new BrowserTypeConnectOptions(connectOptions?.Options ?? new());`
`55`	`55`	`var headers = options.Headers?.ToDictionary(kvp => kvp.Key, kvp => kvp.Value) ?? [];`
`56`	`56`	`headers.Add("x-playwright-launch-options", JsonSerializer.Serialize(PlaywrightSettingsProvider.LaunchOptions, new JsonSerializerOptions() { DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull }));`
`57`	`57`	`options.Headers = headers;`
`58`		`- return await browserType.ConnectAsync(connectOptions.WSEndpoint, options).ConfigureAwait(false);`
	`58`	`+ return await browserType.ConnectAsync(connectOptions.Value.WSEndpoint, options).ConfigureAwait(false);`
`59`	`59`	`}`
`60`	`60`
`61`	`61`	`var legacyBrowser = await ConnectBasedOnEnv(browserType);`
Original file line number	Diff line number	Diff line change
`@@ -62,8 +62,5 @@ public async Task BrowserTearDown()`
`62`	`62`	`Browser = null!;`
`63`	`63`	`}`
`64`	`64`
`65`		`- public virtual Task<PlaywrightConnectOptions?> ConnectOptionsAsync()`
`66`		`- {`
`67`		`- return Task.FromResult<PlaywrightConnectOptions?>(null);`
`68`		`- }`
	`65`	`+ public virtual Task<(string, BrowserTypeConnectOptions?)?> ConnectOptionsAsync() => null!;`
`69`	`66`	`}`
Original file line number	Diff line number	Diff line change
`@@ -61,8 +61,5 @@ public async Task BrowserTearDown()`
`61`	`61`	`Browser = null!;`
`62`	`62`	`}`
`63`	`63`
`64`		`- public virtual Task<PlaywrightConnectOptions?> ConnectOptionsAsync()`
`65`		`- {`
`66`		`- return Task.FromResult<PlaywrightConnectOptions?>(null);`
`67`		`- }`
	`64`	`+ public virtual Task<(string, BrowserTypeConnectOptions?)?> ConnectOptionsAsync() => null!;`
`68`	`65`	`}`
Original file line number	Diff line number	Diff line change
`@@ -496,6 +496,7 @@ test.describe('ConnectOptions', () => {`
`496`	`496`	const ExampleTestWithConnectOptions = `
`497`	`497`	`using System;`
`498`	`498`	`using System.Threading.Tasks;`
	`499`	`+ using Microsoft.Playwright;`
`499`	`500`	`using Microsoft.Playwright.MSTest;`
`500`	`501`	`using Microsoft.VisualStudio.TestTools.UnitTesting;`
`501`	`502`
`@@ -509,11 +510,9 @@ test.describe('ConnectOptions', () => {`
`509`	`510`	`{`
`510`	`511`	`await Page.GotoAsync("about:blank");`
`511`	`512`	`}`
`512`		`- public override PlaywrightConnectOptions ConnectOptions()`
	`513`	`+ public override async Task<(string, BrowserTypeConnectOptions)?> ConnectOptionsAsync()`
`513`	`514`	`{`
`514`		`- return new() {`
`515`		`- WSEndpoint = "http://127.0.0.1:1234",`
`516`		`- };`
	`515`	`+ return ("http://127.0.0.1:1234", null);`
`517`	`516`	`}`
`518`	`517`	}`;
`519`	`518`
Original file line number	Diff line number	Diff line change
`@@ -492,6 +492,7 @@ test.describe('ConnectOptions', () => {`
`492`	`492`	const ExampleTestWithConnectOptions = `
`493`	`493`	`using System;`
`494`	`494`	`using System.Threading.Tasks;`
	`495`	`+ using Microsoft.Playwright;`
`495`	`496`	`using Microsoft.Playwright.NUnit;`
`496`	`497`	`using NUnit.Framework;`
`497`	`498`
`@@ -505,11 +506,9 @@ test.describe('ConnectOptions', () => {`
`505`	`506`	`await Page.GotoAsync("about:blank");`
`506`	`507`	`}`
`507`	`508`
`508`		`- public override PlaywrightConnectOptions ConnectOptions()`
	`509`	`+ public override async Task<(string, BrowserTypeConnectOptions)?> ConnectOptionsAsync()`
`509`	`510`	`{`
`510`		`- return new() {`
`511`		`- WSEndpoint = "http://127.0.0.1:1234",`
`512`		`- };`
	`511`	`+ return ("http://127.0.0.1:1234", null);`
`513`	`512`	`}`
`514`	`513`	}`;
`515`	`514`
Original file line number	Diff line number	Diff line change
`@@ -542,21 +542,22 @@ test.describe('ConnectOptions', () => {`
`542`	`542`	const ExampleTestWithConnectOptions = `
`543`	`543`	`using System;`
`544`	`544`	`using System.Threading.Tasks;`
	`545`	`+ using Microsoft.Playwright;`
`545`	`546`	`using Microsoft.Playwright.Xunit;`
`546`	`547`	`using Xunit;`
	`548`	`+`
`547`	`549`	`namespace Playwright.TestingHarnessTest.Xunit;`
	`550`	`+`
`548`	`551`	`public class <class-name> : PageTest`
`549`	`552`	`{`
`550`	`553`	`[Fact]`
`551`	`554`	`public async Task Test()`
`552`	`555`	`{`
`553`	`556`	`await Page.GotoAsync("about:blank");`
`554`	`557`	`}`
`555`		`- public override PlaywrightConnectOptions ConnectOptions()`
	`558`	`+ public override async Task<(string, BrowserTypeConnectOptions)?> ConnectOptionsAsync()`
`556`	`559`	`{`
`557`		`- return new() {`
`558`		`- WSEndpoint = "http://127.0.0.1:1234",`
`559`		`- };`
	`560`	`+ return ("http://127.0.0.1:1234", null);`
`560`	`561`	`}`
`561`	`562`	}`;
`562`	`563`
Original file line number	Diff line number	Diff line change
`@@ -60,8 +60,5 @@ public override async Task DisposeAsync()`
`60`	`60`	`await base.DisposeAsync().ConfigureAwait(false);`
`61`	`61`	`}`
`62`	`62`
`63`		`- public virtual Task<PlaywrightConnectOptions?> ConnectOptionsAsync()`
`64`		`- {`
`65`		`- return Task.FromResult<PlaywrightConnectOptions?>(null);`
`66`		`- }`
	`63`	`+ public virtual Task<(string, BrowserTypeConnectOptions?)?> ConnectOptionsAsync() => null!;`
`67`	`64`	`}`