review feedback

mxschmitt · mxschmitt · commit 86fea44bfb4c · 2025-01-30T15:35:45.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
@@ -29,7 +29,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
@@ -520,6 +520,7 @@ test.describe('ConnectOptions', () => {
   const ExampleTestWithConnectOptions = `
     using System;
     using System.Threading.Tasks;
+    using Microsoft.Playwright;
     using Microsoft.Playwright.MSTest;
     using Microsoft.VisualStudio.TestTools.UnitTesting;
 
@@ -533,11 +534,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
@@ -513,6 +513,7 @@ test.describe('ConnectOptions', () => {
   const ExampleTestWithConnectOptions = `
     using System;
     using System.Threading.Tasks;
+    using Microsoft.Playwright;
     using Microsoft.Playwright.NUnit;
     using NUnit.Framework;
 
@@ -526,11 +527,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
@@ -563,21 +563,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
`@@ -520,6 +520,7 @@ test.describe('ConnectOptions', () => {`
`520`	`520`	const ExampleTestWithConnectOptions = `
`521`	`521`	`using System;`
`522`	`522`	`using System.Threading.Tasks;`
	`523`	`+ using Microsoft.Playwright;`
`523`	`524`	`using Microsoft.Playwright.MSTest;`
`524`	`525`	`using Microsoft.VisualStudio.TestTools.UnitTesting;`
`525`	`526`
`@@ -533,11 +534,9 @@ test.describe('ConnectOptions', () => {`
`533`	`534`	`{`
`534`	`535`	`await Page.GotoAsync("about:blank");`
`535`	`536`	`}`
`536`		`- public override PlaywrightConnectOptions ConnectOptions()`
	`537`	`+ public override async Task<(string, BrowserTypeConnectOptions)?> ConnectOptionsAsync()`
`537`	`538`	`{`
`538`		`- return new() {`
`539`		`- WSEndpoint = "http://127.0.0.1:1234",`
`540`		`- };`
	`539`	`+ return ("http://127.0.0.1:1234", null);`
`541`	`540`	`}`
`542`	`541`	}`;
`543`	`542`
Original file line number	Diff line number	Diff line change
`@@ -513,6 +513,7 @@ test.describe('ConnectOptions', () => {`
`513`	`513`	const ExampleTestWithConnectOptions = `
`514`	`514`	`using System;`
`515`	`515`	`using System.Threading.Tasks;`
	`516`	`+ using Microsoft.Playwright;`
`516`	`517`	`using Microsoft.Playwright.NUnit;`
`517`	`518`	`using NUnit.Framework;`
`518`	`519`
`@@ -526,11 +527,9 @@ test.describe('ConnectOptions', () => {`
`526`	`527`	`await Page.GotoAsync("about:blank");`
`527`	`528`	`}`
`528`	`529`
`529`		`- public override PlaywrightConnectOptions ConnectOptions()`
	`530`	`+ public override async Task<(string, BrowserTypeConnectOptions)?> ConnectOptionsAsync()`
`530`	`531`	`{`
`531`		`- return new() {`
`532`		`- WSEndpoint = "http://127.0.0.1:1234",`
`533`		`- };`
	`532`	`+ return ("http://127.0.0.1:1234", null);`
`534`	`533`	`}`
`535`	`534`	}`;
`536`	`535`
Original file line number	Diff line number	Diff line change
`@@ -563,21 +563,22 @@ test.describe('ConnectOptions', () => {`
`563`	`563`	const ExampleTestWithConnectOptions = `
`564`	`564`	`using System;`
`565`	`565`	`using System.Threading.Tasks;`
	`566`	`+ using Microsoft.Playwright;`
`566`	`567`	`using Microsoft.Playwright.Xunit;`
`567`	`568`	`using Xunit;`
	`569`	`+`
`568`	`570`	`namespace Playwright.TestingHarnessTest.Xunit;`
	`571`	`+`
`569`	`572`	`public class <class-name> : PageTest`
`570`	`573`	`{`
`571`	`574`	`[Fact]`
`572`	`575`	`public async Task Test()`
`573`	`576`	`{`
`574`	`577`	`await Page.GotoAsync("about:blank");`
`575`	`578`	`}`
`576`		`- public override PlaywrightConnectOptions ConnectOptions()`
	`579`	`+ public override async Task<(string, BrowserTypeConnectOptions)?> ConnectOptionsAsync()`
`577`	`580`	`{`
`578`		`- return new() {`
`579`		`- WSEndpoint = "http://127.0.0.1:1234",`
`580`		`- };`
	`581`	`+ return ("http://127.0.0.1:1234", null);`
`581`	`582`	`}`
`582`	`583`	}`;
`583`	`584`