microsoft / playwright-dotnet

.NET version of the Playwright testing and automation library.
https://playwright.dev/dotnet/
MIT License
2.47k stars 235 forks source link

Publishing to specific runtime flattens native directory structure #1145

Closed Kukkimonsuta closed 3 years ago

Kukkimonsuta commented 3 years ago

When publishing project referencing playwright-sharp the driver is installed to runtimes/<platform>/native folder, however when publishing to specific runtime there is no runtimes/<platform>/native folder and all the contents get copied directly into publish directory and don't preserve any further directory structure breaking the project.

Tested under .NET 5 (see sample) and .NET Core 3.1, behavior is the same.

Primitive sample here: test-playwright-publish.zip

dotnet publish => OK

image image

dotnet publish --runtime win-x64 => flattened, app cannot start

image

kblok commented 3 years ago

Thanks for the report @Kukkimonsuta. I'll take a look at that.

zarenner commented 3 years ago

I just ran into this as well.

FYI @kblok, per https://github.com/dotnet/sdk/issues/9643#issuecomment-426767189 this (arguably broken IMO) behavior appears to possibly be by design in dotnet. I haven't investigated further to see if there's a reasonable workaround either on the production or consumption side, although perhaps platform-specific dependency packages w/ contentFiles are an option here.

edit: Filed https://github.com/NuGet/docs.microsoft.com-nuget/issues/2322 requesting the dotnet/nuget teams document this behavior.

kblok commented 3 years ago

Thanks for reporting that @zarenner. We will see how we can get out of that. I bet we will need to ship the zip file in the meantime.

avodovnik commented 3 years ago

@Kukkimonsuta thanks for the great feedback!

We've had a think about what we can do to resolve this, and one of the ideas, that also fits with our near-term plans, is to:

  1. Move the driver part to contentFiles as suggested above as an approach, and
  2. Deliver the Browsers separately in individual NuGet packages (i.e. PlaywrightSharp.Chromium, PlaywrightSharp.Firefox, etc.) in a similar fashion

The benefits here would be that there's no more downloading of artifacts at build-time, which is something we've been talking about moving towards for the past few months. Also, it seems the problem above would also be solved with this approach.

There is a downside however, specifically, we feel strongly against creating a package per platform. We believe the workflow of write anywhere, publish to anywhere does make sense. That means that each package would carry some overheard (in terms of browsers & runtimes built for all platforms). An approach we're discussing to mitigate that would be to also ship a build-task that customers can include, to delete non-targeted platforms as a build task.

We'll work on getting this fix (at least some part of it) to align with v1.9, so it should be ready "soon"-ish, hopefully.

Let us know if you (or anyone else) have any comments, hesitations or improvements!

Thanks, Anze

Kukkimonsuta commented 3 years ago

Both moving driver to contentFiles and browsers to nuget packages makes sense - not only gets rid of downloading artifacts at build/run time, but it as well ensures everything that is needed to run the project comes from one source - NuGet - which is great as well.

Big downside however is that the size will explode if we're going to be including all browsers for all platforms (including just the driver now is already quite a lot), so I'd like to propose one additional change - allow moving driver and browsers to a dotnet tool. NuGet hierarchy would look something like this:

- PlaywrightSharp
    - PlaywrightSharp.Core
    - PlaywrightSharp.Driver
    - PlaywrightSharp.Browser.Chromium
    - PlaywrightSharp.Browser.Firefox
    - PlaywrightSharp.Browser.<etc>
- dotnet-playwrightsharp

By default nothing would be different from what you're proposing - user would install PlaywrightSharp nuget package and have everything ready. However there would also be option to install just PlaywrightSharp.Core which wouldn't contain any of the native dependencies (driver/browsers) and instead rely on them to be already installed in the system. For that there would be dotnet-playwrightsharp. Note that it wouldn't depend on any of the driver/browsers as the tool will need to able to hold multiple driver/browsers versions since the intention is to have it installed globally and for these dependencies to be reused by any apps that require them. The tool would interact with nuget to download and install whatever dependencies are required. Downside of this approach would be runtime error if requested driver/browser isn't installed and would probably require user intervention, but since it's opt-in behavior it would be fully up to the user whether that's acceptable or not - I'd certainly use it to avoid hundreds of extra megabytes moving through our deployment pipelines every time.

If you're interested in supporting this scenario, but don't have enough resources I could help out building the tool side of things.

avodovnik commented 3 years ago

Yeah, that's a great suggestion and reasonable to implement. I'm not entirely convinced yet on the the dotnet tool. There's a bunch of challenges that @kblok already went through before, so the pain is still fresh in his memory :-).

We'll go about fixing the problem first, and improving the solution after that. PRs are and will definitely be welcomed!

avodovnik commented 3 years ago

Quick update on this issue, I tried the approach of using contentFiles and that proved catastrophic. It was incredibly slow. I suspect that's due to how it's designed in the background with the project system adding links to those files, one by one, essentially. So, instead of going down that route, we've adapted the approach from mono and decided to add a special msbuild target/step where it copies over all the files (CopyPlaywrightFilesToOutput) to a new folder .playwright. To unblock this particular bug, we've decided to not clean-up non-platform-specific folders, so you might see unix, osx, etc., on published builds. This will change in the future.

The rest, currently, stayed the same (i.e. browsers still get downloaded at build, etc.). That's something we'll be addressing soon, as well.