[wasm] Sanity check generated nuget packages

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.

https://docs.microsoft.com/dotnet/core/

MIT License

15.18k stars 4.72k forks source link

[wasm] Sanity check generated nuget packages #50198

Closed radical closed 3 years ago

radical commented 3 years ago

We recently had a regression where mono-aot-cross.exe got dropped from the cross compiler package on windows. We should have some basic checks like validating the list of included files against an expected list, to guard against this in the future.

AFAICS, we don't have anything for that right now.

/cc @akoeplinger @steveisok @lewing

ghost commented 3 years ago

Tagging subscribers to 'arch-wasm': @lewing See info in area-owners.md if you want to be subscribed.

Issue Details

We recently had a regression where `mono-aot-cross.exe` got dropped from the cross compiler package on windows. We should have some basic checks like validating the list of included files against an expected list, to guard against this in the future. AFAICS, we don't have anything for that right now. /cc @akoeplinger @steveisok @lewing

Author:	radical
Assignees:	-
Labels:	`arch-wasm`, `area-Build-mono`
Milestone:	-

lewing commented 3 years ago

@steveisok it happened again 🤦 can we get some validation setup this time?

lewing commented 3 years ago


/usr/local/share/dotnet/packs/Microsoft.NET.Runtime.WebAssembly.Sdk/6.0.0-preview.5.21262.1/Sdk/WasmApp.targets(358,5): error : Could not find AOT cross compiler at $(MonoAotCrossCompilerPath)=/usr/local/share/dotnet/packs/Microsoft.NETCore.App.Runtime.AOT.osx-x64.Cross.browser-wasm/6.0.0-preview.5.21262.1/Sdk/../tools/mono-aot-cross [/Users/lewing/Source/bsize/bsize.csproj]```

radical commented 3 years ago

Would the tests under https://github.com/dotnet/runtime/tree/main/src/libraries/pkg/test be suitable for this? IIUC, we would add a targets file under src/libraries/pkg/test/packageSettings/$packageId/, which could then do the check.

steveisok commented 3 years ago

@directhex does it make sense to add pkg tests here?

eerhardt commented 3 years ago

I would think we would want tests that tested the end-to-end experience like a customer would use the product. That way, we can ensure all the parts work together correctly.

Given that reasoning, I think the tests would probably live in dotnet/sdk or dotnet/installer. We have similar tests in those repos that test the end-to-end, for example for Blazor WASM apps.

https://github.com/dotnet/sdk/blob/23e7fd3c08395b368f7db8bee30c20c558b29bd4/src/Tests/Microsoft.NET.Sdk.BlazorWebAssembly.Tests/WasmPublishTest.cs#L25

steveisok commented 3 years ago

Agreed. I would also like to test coming out of runtime as certain small build tweaks can end up causing sizable regressions.

eerhardt commented 3 years ago

lewing commented 3 years ago

Given that reasoning, I think the tests would probably live in dotnet/sdk or dotnet/installer. We have similar tests in those repos that test the end-to-end, for example for Blazor WASM apps.

The Manifest is consumed by directly by installer as part of the baseline manifests so that it probably a good place for an e2e check

directhex commented 3 years ago

This is resolved by https://github.com/dotnet/runtime/pull/52784

eerhardt commented 3 years ago

This is resolved by #52784

I don't see tests being added with that change. What are we doing to ensure this doesn't regress again?