Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(playwrighttesting): update public classes, methods and constants as per APIView review #47652

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

chore(playwrighttesting): update public classes, methods and constants as per APIView review #47652

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ab6765f
chore(): renamed Os to OS
Dec 23, 2024
cafb573
chore(): convert TestReportingClientOptions to internal class
Dec 23, 2024
3b862e9
chore(): refactor ConnectOptions class
Dec 23, 2024
a883f49
chore(): use extensible enums
Dec 24, 2024
4b8b31f
chore(): add sync and async methods for Initialize
Dec 24, 2024
47aa940
chore(): add logging via Microsoft.Extensions.Logging
Dec 24, 2024
41d1a1a
chore(): update public api code
Dec 26, 2024
b66cddd
chore(): update Microsoft.Extensions.Logging to 8.x
Jan 1, 2025
dc8b580
chore(): bump dependencies to 8.x
Jan 3, 2025
1c4e6f0
Merge branch 'main' of https://github.com/Sid200026/azure-sdk-for-net…
Jan 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions eng/Packages.Data.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@
<ItemGroup Condition="$(MSBuildProjectName.StartsWith('Azure.Developer.MicrosoftPlaywrightTesting'))">
<PackageReference Update="Microsoft.TestPlatform.ObjectModel" Version="17.10.0" />
<PackageReference Update="NUnit" Version="3.13.2" />
<PackageReference Update="Microsoft.Extensions.Logging" Version="2.0.0" />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added as per recommendation in APIView review to use Microsoft.Extensions.Logging's ILogger interface. Added the dependency under this condition since it's used only in Extension libraries.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be 8.x unless there's a reason to not do so. We're in the process of bumping all of the Azure SDK dependencies to the 8.x line.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Microsoft.Extensions.Logging 8.x uses Microsoft.Bcl.AsyncInterfaces 8.x, while in Packages.Data.props, Microsoft.Bcl.AsyncInterfaces is set to 6.x -

azure-sdk-for-net/eng/Packages.Data.props

Line 104 in f97d3dd

<PackageReference Update="Microsoft.Bcl.AsyncInterfaces" Version="6.0.0" />

This causes a version mismatch in net6.0

C:\Program Files\dotnet\sdk\8.0.404\Microsoft.Common.CurrentVersion.targets(2412,5): warning MSB3277: Found conflicts b
etween different versions of "Microsoft.Bcl.AsyncInterfaces" that could not be resolved. [Dev\azure
-sdk-for-net\sdk\playwrighttesting\Azure.Developer.MicrosoftPlaywrightTesting.TestLogger\tests\Azure.Developer.Microsof
tPlaywrightTesting.TestLogger.Tests.csproj::TargetFramework=net6.0]

Copy link
Member

@jsquire jsquire Dec 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Async Interfaces package is in the process of being bumped to the 8.x line centrally, along with the remainder of our dependencies. Please either wait for that to complete or bump the necessary dependencies for Playwright. I'd rather not see us use an older set here that will need to be migrated separately.

A direct reference to the Async Interfaces packages locally should resolve the conflict in the interim.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes a version mismatch in net6.0

.NET 6 reached end-of-life and is no longer a supported platform. Our test matrices are also in the process of being upgraded to remove 6 and include 9.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A direct reference to the Async Interfaces packages locally should resolve the conflict in the interim.

Do we add Microsoft.Bcl.AsyncInterfaces in the csproj with version as 8.x?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just rechecked the dotnet build logs, the above were warnings rather than errors.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we add Microsoft.Bcl.AsyncInterfaces in the csproj with version as 8.x?

You would add the package reference in your .csproj and add a version of 8.x in your Playwright section of the central packages file. That central version will get cleaned up when we complete the repo-wide central version bumps.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To resolve the issue with net6.0, have added the below dependencies in the central package file under the Playwright section.

    <PackageReference Update="Microsoft.Bcl.AsyncInterfaces" Version="8.0.0" />
    <PackageReference Update="System.Diagnostics.DiagnosticSource" Version="8.0.0" />

Also added reference to these packages in our base SDK's .csproj.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

//fyi: @m-redding

</ItemGroup>

<ItemGroup Condition="$(MSBuildProjectName.Contains('CloudMachine'))">
Expand Down
1 change: 1 addition & 0 deletions ...rosoftPlaywrightTesting.NUnit/src/Azure.Developer.MicrosoftPlaywrightTesting.NUnit.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,6 @@
<ItemGroup>
<ProjectReference Include="..\..\Azure.Developer.MicrosoftPlaywrightTesting.TestLogger\src\Azure.Developer.MicrosoftPlaywrightTesting.TestLogger.csproj" />
<PackageReference Include="NUnit" />
<PackageReference Include ="Microsoft.Extensions.Logging" />
</ItemGroup>
</Project>
31 changes: 0 additions & 31 deletions ...righttesting/Azure.Developer.MicrosoftPlaywrightTesting.NUnit/src/NUnitFrameworkLogger.cs
View file
Open in desktop

This file was deleted.

70 changes: 70 additions & 0 deletions sdk/playwrighttesting/Azure.Developer.MicrosoftPlaywrightTesting.NUnit/src/NUnitLogger.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using Microsoft.Extensions.Logging;
using NUnit.Framework;

namespace Azure.Developer.MicrosoftPlaywrightTesting.NUnit;

internal class NUnitLogger : ILogger
{
public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func<TState, Exception?, string> formatter)
{
if (!IsEnabled(logLevel))
{
return;
}
if (formatter == null)
{
throw new ArgumentNullException(nameof(formatter));
}
string message = formatter(state, exception);
if (exception != null)
{
message += $"\nException: {exception}";
}
Log(logLevel, message);
}

public bool IsEnabled(LogLevel logLevel)
{
return true;
}

IDisposable? ILogger.BeginScope<TState>(TState state)
{
return NullScope.Instance;
}

private static void Log(LogLevel level, string message)
{
System.IO.TextWriter writer = (level == LogLevel.Error || level == LogLevel.Warning || level == LogLevel.Critical) ? Console.Error : Console.Out;
writer.WriteLine($"{DateTime.Now} [{level}]: {message}");

if (level == LogLevel.Debug)
{
TestContext.WriteLine($"[MPT-NUnit]: {message}");
}
else if (level == LogLevel.Error || level == LogLevel.Critical)
{
TestContext.Error.WriteLine($"[MPT-NUnit]: {message}");
}
else
{
TestContext.Progress.WriteLine($"[MPT-NUnit]: {message}");
}
}
};

internal class NullScope : IDisposable
{
public static readonly NullScope Instance = new();

private NullScope() { }

public void Dispose()
{
// No operation
}
}
30 changes: 17 additions & 13 deletions ...ghttesting/Azure.Developer.MicrosoftPlaywrightTesting.NUnit/src/PlaywrightServiceNUnit.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
using System.Runtime.InteropServices;
using System;
using Azure.Developer.MicrosoftPlaywrightTesting.TestLogger;
using Azure.Developer.MicrosoftPlaywrightTesting.TestLogger.Interface;
using Microsoft.Extensions.Logging;

namespace Azure.Developer.MicrosoftPlaywrightTesting.NUnit;

Expand All @@ -17,28 +17,32 @@ namespace Azure.Developer.MicrosoftPlaywrightTesting.NUnit;
[SetUpFixture]
public class PlaywrightServiceNUnit : PlaywrightService
{
private static NUnitFrameworkLogger nunitFrameworkLogger { get; } = new();
private static NUnitLogger nunitLogger { get; } = new();
private static readonly string? s_useCloudHostedBrowsers = TestContext.Parameters.Get(RunSettingKey.UseCloudHostedBrowsers.ToString());
private static readonly string? s_serviceAuthType = TestContext.Parameters.Get(RunSettingKey.ServiceAuthType.ToString());
/// <summary>
/// Initializes a new instance of the <see cref="PlaywrightServiceNUnit"/> class.
/// </summary>
/// <param name="credential">The azure token credential to use for authentication.</param>
public PlaywrightServiceNUnit(TokenCredential? credential = null)
: base(playwrightServiceOptions, credential: credential, frameworkLogger: nunitFrameworkLogger)
: base(credential, options)
{
}

/// <summary>
/// Creates a new instance of <see cref="PlaywrightServiceOptions"/> based on the runsettings file.
/// </summary>
public static PlaywrightServiceOptions playwrightServiceOptions { get; } = new(
os: GetOsPlatform(TestContext.Parameters.Get(RunSettingKey.Os)),
runId: TestContext.Parameters.Get(RunSettingKey.RunId),
exposeNetwork: TestContext.Parameters.Get(RunSettingKey.ExposeNetwork),
serviceAuth: TestContext.Parameters.Get(RunSettingKey.ServiceAuthType),
useCloudHostedBrowsers: TestContext.Parameters.Get(RunSettingKey.UseCloudHostedBrowsers),
azureTokenCredentialType: TestContext.Parameters.Get(RunSettingKey.AzureTokenCredentialType),
managedIdentityClientId: TestContext.Parameters.Get(RunSettingKey.ManagedIdentityClientId)
);
public static PlaywrightServiceOptions options { get; } = new()
{
OS = GetOsPlatform(TestContext.Parameters.Get(RunSettingKey.OS.ToString())),
RunId = TestContext.Parameters.Get(RunSettingKey.RunId.ToString()),
ExposeNetwork = TestContext.Parameters.Get(RunSettingKey.ExposeNetwork.ToString()),
ServiceAuth = string.IsNullOrEmpty(s_serviceAuthType) ? default : new ServiceAuthType(s_serviceAuthType!),
UseCloudHostedBrowsers = !string.IsNullOrEmpty(s_useCloudHostedBrowsers) && bool.Parse(s_useCloudHostedBrowsers),
TokenCredentialType = TestContext.Parameters.Get(RunSettingKey.AzureTokenCredentialType.ToString()),
ManagedIdentityClientId = TestContext.Parameters.Get(RunSettingKey.ManagedIdentityClientId.ToString()),
Logger = nunitLogger
};

/// <summary>
/// Setup the resources utilized by Playwright service.
Expand All @@ -49,7 +53,7 @@ public async Task SetupAsync()
{
if (!UseCloudHostedBrowsers)
return;
nunitFrameworkLogger.Info("\nRunning tests using Microsoft Playwright Testing service.\n");
nunitLogger.LogInformation("\nRunning tests using Microsoft Playwright Testing service.\n");

await InitializeAsync().ConfigureAwait(false);
}
Expand Down
1 change: 1 addition & 0 deletions ...wrightTesting.TestLogger/src/Azure.Developer.MicrosoftPlaywrightTesting.TestLogger.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@
<PackageReference Include="Azure.Storage.Blobs" />
<PackageReference Include="System.IdentityModel.Tokens.Jwt" />
<PackageReference Include="Microsoft.TestPlatform.ObjectModel" />
<PackageReference Include="Microsoft.Extensions.Logging" />
</ItemGroup>
</Project>
2 changes: 1 addition & 1 deletion ....Developer.MicrosoftPlaywrightTesting.TestLogger/src/Client/TestReportingClientOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
namespace Azure.Developer.MicrosoftPlaywrightTesting.TestLogger.Client
{
/// <summary> Client options for TestReportingClient. </summary>
public partial class TestReportingClientOptions : ClientOptions
internal partial class TestReportingClientOptions : ClientOptions
{
private const ServiceVersion LatestVersion = ServiceVersion.V2024_09_01_Preview;

Expand Down
11 changes: 8 additions & 3 deletions ...wrighttesting/Azure.Developer.MicrosoftPlaywrightTesting.TestLogger/src/ConnectOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,21 @@ namespace Azure.Developer.MicrosoftPlaywrightTesting.TestLogger;
/// Represents the connect options for a generic type.
/// </summary>
/// <typeparam name="T">The type parameter.</typeparam>
public class ConnectOptions<T> where T : class, new()
/// <remarks>
/// Initializes a new instance of the <see cref="ConnectOptions{T}"/> class.
/// </remarks>
/// <param name="WsEndpoint"></param>
/// <param name="Options"></param>
public class ConnectOptions<T>(string WsEndpoint, T Options) where T : class, new()
{
/// <summary>
/// A browser websocket endpoint to connect to.
/// </summary>
public string? WsEndpoint { get; set; }
public string WsEndpoint { get; set; } = WsEndpoint;
/// <summary>
/// Connect options for the service.
/// </summary>
public T? Options { get; set; }
public T Options { get; set; } = Options;
}

internal class BrowserConnectOptions
Expand Down
Loading
Loading