Enabled Baseline Package Validation

[Julian Verdurmen (304NotModified)]

see #3161 (comment)

• Enabled Package baseline validation, see https://learn.microsoft.com/en-us/dotnet/fundamentals/apicompat/package-validation/baseline-version-validator
• Use 1.52.0 as baseline (as discussed below)
• Added CompatibilitySuppressions.xml for breaking changes sinds 1.52 (discussed below)
• CLI package is not in scope
• Added CP0006 - A member was added to an interface without a default implementation (see see https://learn.microsoft.com/en-us/dotnet/fundamentals/apicompat/diagnostic-ids) to <NoWarn> on Playwright.csproj, as that is the normal flow?
• Add a new workflow for checking the package results. Another option is to add <GeneratePackageOnBuild>True</GeneratePackageOnBuild> to the csproj files
  • "NuGet Package Smoke Test" is not testing all the packages and have a different goal
• Example of breakage: Package baseline validation (breaking example) #3170

[Max Schmitt (mxschmitt)]

It looks great but I'm asking myself if its worth using a very old version as a baseline. I'd instead use a more recent one and then continue the journey from there?

[Julian Verdurmen (304NotModified)]

As we do semver, we would like to know the breaking changes when someone upgrades from a old version?

Some breaking changes are acceptable, and those are in the suppression file :)

[Julian Verdurmen (304NotModified)]

It seems we have already new breaking changes and not sure if they are intentional 😅

[Julian Verdurmen (304NotModified)]

Max Schmitt (@mxschmitt)
So this is the new breaking change in master (introduced by d0f2745)

First we had a public empty ctor for BindingSource.cs, now that ctor is gone!

Is that intentional?

Anyway, shown by the baseline package validation :)

I've added it to the suppressions by dotnet pack /p:ApiCompatGenerateSuppressionFile=true

[Max Schmitt (mxschmitt)]

First we had a public empty ctor for BindingSource.cs, now that ctor is gone!

Yes, because BindingSource is a class we construct and not the user.

I think lets consider everything before the latest release as "it was intentional" instead of committing a bunch of irrelevant exceptions to the repository. Do you mind adjusting it? then we can merge.

[Julian Verdurmen (304NotModified)]

So the baseline version should 1.52.0, correct?

[Max Schmitt (mxschmitt)]

So the baseline version should 1.52.0, correct?

Yes

[Julian Verdurmen (304NotModified)]

OK working on it :)

[Julian Verdurmen (304NotModified)]

Max Schmitt (@mxschmitt)

Updated, but there is also a new (small) breaking change?

From #3184 - the Ref property is removed from LocatorAriaSnapshotOptions.cs

But it's documented a new in 1.52, so it just introduced.

https://playwright.dev/dotnet/docs/api/class-locator#locator-aria-snapshot

Anyway, if this in indented, then this PR is ready

[Max Schmitt (mxschmitt)]

Awesome! Yeah the Ref property was intentionally removed.

[Julian Verdurmen (304NotModified)]

All green. Feel free to rename the PR before merge :)

[Darío Kondratiuk (kblok)]

Julian Verdurmen (@304NotModified) Max Schmitt (@mxschmitt) are we semver? I think playwright broke compatibility many times.

[Julian Verdurmen (304NotModified)]

Darío Kondratiuk (@kblok)

I hope this PR will help in getting the API more stable in terms of breaking changes.
Sometime there are good reasons to break compatibility, this is now more transparent and explicit.

My report of the binary breaking change (#3161) was quickly fixed, so thanks for that! I hope we have less need for these fixes in the future :)

[Darío Kondratiuk (kblok)]

Julian Verdurmen (@304NotModified) The problem is that upstream playwright determines the API, andplaywright-dotnet matches the upstream version. If upstream breaks the compatibility without changing the major version, there is nothing we can do here.

[Julian Verdurmen (304NotModified)]

Julian Verdurmen (@304NotModified) The problem is that upstream playwright determines the API, andplaywright-dotnet matches the upstream version. If upstream breaks the compatibility without changing the major version, there is nothing we can do here.

Darío Kondratiuk (@kblok)
I don't think that's always true. E.g. instead of removing the Ref property (see #3169 (comment)) we could mark it with a [Obsolete] attribuut. Now it's a binary and source breaking change.