review feedback

microsoft · Jan 30, 2025 · 86fea44 · 86fea44
1 parent b3747fc
commit 86fea44
Show file tree

Hide file tree

Showing 11 changed files with 30 additions and 70 deletions.
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!;
 }