From c591630ec204fb88d336cfedfa492dbe0a254d6a Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Thu, 30 Jan 2025 15:35:45 +0100 Subject: [PATCH] review feedback --- src/Playwright.MSTest/BrowserService.cs | 10 +++---- src/Playwright.MSTest/BrowserTest.cs | 5 +--- src/Playwright.NUnit/BrowserService.cs | 10 +++---- src/Playwright.NUnit/BrowserTest.cs | 5 +--- .../PlaywrightConnectOptions.cs | 30 ------------------- .../tests/baseTest.ts | 2 +- .../tests/mstest/basic.spec.ts | 7 ++--- .../tests/nunit/basic.spec.ts | 7 ++--- .../tests/xunit/basic.spec.ts | 9 +++--- src/Playwright.Xunit/BrowserService.cs | 10 +++---- src/Playwright.Xunit/BrowserTest.cs | 5 +--- 11 files changed, 30 insertions(+), 70 deletions(-) delete mode 100644 src/Playwright.TestAdapter/PlaywrightConnectOptions.cs diff --git a/src/Playwright.MSTest/BrowserService.cs b/src/Playwright.MSTest/BrowserService.cs index 99526e170..040b141c4 100644 --- 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 Register(WorkerAwareTest test, IBrowserType browserType, PlaywrightConnectOptions? connectOptions) + public static Task 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 CreateBrowser(IBrowserType browserType, PlaywrightConnectOptions? connectOptions) + private static async Task 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 index f7ab38338..13a8d30b1 100644 --- 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 ConnectOptionsAsync() - { - return Task.FromResult(null); - } + public virtual Task<(string, BrowserTypeConnectOptions?)?> ConnectOptionsAsync() => null!; } diff --git a/src/Playwright.NUnit/BrowserService.cs b/src/Playwright.NUnit/BrowserService.cs index 0574f3e3d..081d5afe9 100644 --- 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 Register(WorkerAwareTest test, IBrowserType browserType, PlaywrightConnectOptions? connectOptions) + public static Task 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 CreateBrowser(IBrowserType browserType, PlaywrightConnectOptions? connectOptions) + private static async Task 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 index 7efdcfbaf..62cc07eb6 100644 --- 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 ConnectOptionsAsync() - { - return Task.FromResult(null); - } + public virtual Task<(string, BrowserTypeConnectOptions?)?> ConnectOptionsAsync() => null!; } diff --git a/src/Playwright.TestAdapter/PlaywrightConnectOptions.cs b/src/Playwright.TestAdapter/PlaywrightConnectOptions.cs deleted file mode 100644 index 881b6d044..000000000 --- a/src/Playwright.TestAdapter/PlaywrightConnectOptions.cs +++ /dev/null @@ -1,30 +0,0 @@ -/* - * MIT License - * - * Copyright (c) Microsoft Corporation. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. - */ - -namespace Microsoft.Playwright.TestAdapter; - -public class PlaywrightConnectOptions : BrowserTypeConnectOptions -{ - public string WSEndpoint = string.Empty; -} diff --git a/src/Playwright.TestingHarnessTest/tests/baseTest.ts b/src/Playwright.TestingHarnessTest/tests/baseTest.ts index cec1cd2f4..024c38d77 100644 --- 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 index 9c92aa2b3..80ef324d1 100644 --- 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 index 19f002f36..495e06432 100644 --- 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 index bf5a33652..d5c2896f9 100644 --- a/src/Playwright.TestingHarnessTest/tests/xunit/basic.spec.ts +++ b/src/Playwright.TestingHarnessTest/tests/xunit/basic.spec.ts @@ -542,9 +542,12 @@ 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 : PageTest { [Fact] @@ -552,11 +555,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.Xunit/BrowserService.cs b/src/Playwright.Xunit/BrowserService.cs index 9b5f46bae..25ffc5a05 100644 --- 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 Register(WorkerAwareTest test, IBrowserType browserType, PlaywrightConnectOptions? connectOptions) + public static Task 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 CreateBrowser(IBrowserType browserType, PlaywrightConnectOptions? connectOptions) + private static async Task 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 index a4b6de758..ae55d55cb 100644 --- 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 ConnectOptionsAsync() - { - return Task.FromResult(null); - } + public virtual Task<(string, BrowserTypeConnectOptions?)?> ConnectOptionsAsync() => null!; }