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

Jump to bottom

Add E2E tests for blazorwasm+workloads+aot #10688

Closed

radical wants to merge 23 commits into dotnet:main from radical:wasm-workloads-aot

Member

radical commented May 20, 2021

builds a blazorwasm app with workload microsoft-net-sdk-blazorwebassembly-aot

radical requested review from lewing, pranavkm and vlada-shubina

May 20, 2021 00:09

radical force-pushed the wasm-workloads-aot branch from 7ff3160 to 6ea42b0 Compare

May 20, 2021 00:15

Member Author

radical commented May 20, 2021

Depends on dotnet/emsdk#19

radical marked this pull request as draft

May 20, 2021 01:05

radical force-pushed the wasm-workloads-aot branch 4 times, most recently from 98019e5 to bbe9144 Compare

May 21, 2021 17:07

Member

lewing commented May 21, 2021 •

edited

Loading

tracking one of the issues dotnet/sdk#17790

Member Author

radical commented May 21, 2021

The tests here install the workload with dotnet workload install .., but since all the tests are using the same dotnet installation (in artifacts), workload install "taints" the manifest, and packs/. Does it sound reasonable to make a fresh copy of the dotnet dir, for every workload test, to avoid this issue? Or is there a better way to test this here?

This would allow tests like:

build blazorwasm with no workload installed
using workload install when it is already installed

@sfoslund @lewing

Member Author

radical commented May 22, 2021

Tests should fail while running emcc --version, because of dotnet/emsdk#19 .

Member

sfoslund commented May 24, 2021

Does it sound reasonable to make a fresh copy of the dotnet dir, for every workload test, to avoid this issue? Or is there a better way to test this here?

This would be hugely costly and add a lot of time to our test runs. @wli3 do you have any suggestions for how to do this or if these tests are needed?

Member Author

radical commented May 24, 2021

Does it sound reasonable to make a fresh copy of the dotnet dir, for every workload test, to avoid this issue? Or is there a better way to test this here?

This would be hugely costly and add a lot of time to our test runs. @wli3 do you have any suggestions for how to do this or if these tests are needed?

Just to be clear, I meant - make a copy of artifacts/bin/redist/Debug/dotnet/.

Member Author

radical commented May 24, 2021

.. and this PR is now using local copies. AFAICS, that itself shouldn't affect run times much. But building for AOT will add some extra time.

pranavkm reviewed

View reviewed changes

pranavkm left a comment

This looks great although I worry about the impact on the build time. FYI @wli3

test/EndToEnd/WorkloadTests.cs Outdated Show resolved Hide resolved

test/EndToEnd/WorkloadTests.cs Show resolved Hide resolved

test/EndToEnd/WorkloadTests.cs Outdated

+                                  + sourceDirName);
+                          }
+                          DirectoryInfo[] dirs = dir.GetDirectories();

pranavkm May 24, 2021

This can be done on Line 124. Also you could consider using EnumerateDirectories()

test/EndToEnd/WorkloadTests.cs Outdated Show resolved Hide resolved

test/EndToEnd/WorkloadTests.cs

+                          DirectoryInfo directory = TestAssets.CreateTestDirectory();
+                          WorkloadTestEnvironment env = PrepareTestEnvironment(directory.FullName);
+                          //FIXME: poke the value in the project file

pranavkm May 24, 2021

What does this mean?

Member Author

radical May 24, 2021

I want to set the property in the project file, instead of forcing it as a global property from the command line. That would be closest to what a user would do.

wli3 reviewed

View reviewed changes

test/EndToEnd/WorkloadTests.cs


		//FIXME: poke the value in the project file

		// AOT builds are slow, so in case the test is failing (IOW, it is able to build)

wli3 May 24, 2021 •

edited

Loading

AOT builds are slow, so in case the test is failing (IOW, it is able to build)

Then it should not be in the installer test. Installer end to end tests are for "if I have all the parts together" with the right version. dotnet/installer does not have parallel test support. And there are a lot of legs. It has less tolerant for flaky tests

Member Author

radical May 25, 2021

This is for testing whether all the parts for the workload, with the right versions work together or not. The tests shouldn't be flaky.

I'll change this to native relinking, which will be much quicker. And we can discuss AOT after that.

Member Author

radical commented May 25, 2021

The python fix in emsdk required for building on linux, is not available here yet:

runtime correctly has the bump to include that
but sdk hasn't taken the bump for runtime yet ([main] Update dependencies from dotnet/sdk #10763),
which means the installer doesn't have that either

I'll add a workaround here for now.

radical force-pushed the wasm-workloads-aot branch 2 times, most recently from bcb9188 to 6079087 Compare

May 29, 2021 00:50

Member Author

radical commented May 29, 2021

Update:

tests still broken on osx - missing emsdk/python fix
linux - workload install throws NRE, that hasn't made it here yet


          Add tests for blazorwasm+aot workload

dc3ab0c

radical force-pushed the wasm-workloads-aot branch from 6079087 to dc3ab0c Compare

May 29, 2021 01:32

radical added 4 commits

June 6, 2021 21:10


          Merge remote-tracking branch 'origin/main' into wasm-workloads-aot

258d828


          Merge remote-tracking branch 'origin/main' into wasm-workloads-aot

5db763b


          Merge remote-tracking branch 'origin/main' into wasm-workloads-aot

e48bf36


          Update test to look for the error number, instead of the error descri…

7dffae5

…ption

Member Author

radical commented Jun 23, 2021

Current state:

The missing workload tests are failing because the error string changed
One of failing on windows because the path to mono-aot-cross.exe is too long (should install packs outside artifacts)
Invariant tests seem to be failing with

      System.UnauthorizedAccessException : Access to the path 'dotnet.exe' is denied.
      Stack Trace:
           at System.IO.FileSystem.RemoveDirectoryRecursive(String fullPath, WIN32_FIND_DATA& findData, Boolean topLevel) in System.Private.CoreLib.dll:token 0x6005c81+0x163
           at System.IO.FileSystem.RemoveDirectory(String fullPath, Boolean recursive) in System.Private.CoreLib.dll:token 0x6005c7e+0x36
        /_/test/Microsoft.DotNet.Tools.Tests.Utilities/Extensions/DirectoryInfoExtensions.cs(33,0): at Microsoft.DotNet.TestFramework.DirectoryInfoExtensions.EnsureExistsAndEmpty(DirectoryInfo subject)
        /_/test/Microsoft.DotNet.Tools.Tests.Utilities/TestAssets.cs(75,0): at Microsoft.DotNet.TestFramework.TestAssets.CreateTestDirectory(String testProjectName, String callingMethod, String identifier)
        /_/test/EndToEnd/WorkloadTests.cs(55,0): at EndToEnd.Tests.WorkloadTests.TestBlazorWasmWithWithWorkload(String extraBuildArgs, String callerName)
        /_/test/EndToEnd/WorkloadTests.cs(51,0): at EndToEnd.Tests.WorkloadTests.ItCanPublishBlazorWasmWithWorkloads_Invariant(String args)

Updated the PR, to fix the first one, and see how it does with main HEAD.


          improve error pattern used for the test

0e32935


          Workaround sdk installation issues on linux, and osx

3b44667

Member Author

radical commented Jun 23, 2021

Current state:

Windows x64:

D:\workspace\_work\1\s\artifacts\bin\EndToEnd.Tests\Release\net6.0\Tests\EndToEnd.Tests\ItCanPublishBlazorWasmWithWorkloads_WithAOT\dotnet\packs\Microsoft.NET.Runtime.WebAssembly.Sdk\6.0.0-preview.7.21321.15\Sdk\WasmApp.Native.targets(342,5):
  error : Could not find AOT cross compiler at $(_MonoAotCrossCompilerPath)=

Windows x86, installing workloads fails with Workload with id microsoft-net-sdk-blazorwebassembly-aot is not supported on this platform., which makes sense
But if you try to build the workload installed:

D:\workspace\_work\1\s\artifacts\bin\EndToEnd.Tests\Debug\net6.0\Tests\EndToEnd.Tests\ItCannotPublishBlazorWasm_AOTWithoutWorkloadInstalled\dotnet\sdk-manifests\6.0.100\microsoft.net.workload.mono.toolchain\WorkloadManifest.targets(50,37):
  error MSB4236: The SDK 'Microsoft.NET.Runtime.Emscripten.Python' specified could not be found.
  
[D:\workspace\_work\1\s\artifacts\bin\EndToEnd.Tests\Debug\net6.0\Tests\EndToEnd.Tests\ItCannotPublishBlazorWasm_AOTWithoutWorkloadInstalled\project\project.csproj]

Member Author

radical commented Jun 23, 2021

Win x64: the cross compiler path issue will be fixed by [wasm] Fix blazor/aot builds runtime#54651
Win x86: Python issue(?)
Linux: Added a workaround for the copy-across-mounts issue Workload install copying across mounts error sdk#18450.
- And now it hits the cross compiler permission issue [mono][workload] Cross compiler pack fails to install on browser due to permission setting runtime#54612
OSX: disabled due to [mono][workload] Cross compiler pack fails to install on browser due to permission setting runtime#54612

Member Author

radical commented Jun 24, 2021

@wli3 what would be the best way to skip tests on win/x86? Add support in one of the attributes, or can we use Microsoft.DotNet.XUnitExtensions? It had conflicts with existing type names when I tried to include it.

radical added 10 commits

June 28, 2021 11:41


          Merge remote-tracking branch 'origin/main' into wasm-workloads-aot

4108a16


          re-enable osx workload tests

d97adc2


          try skipping tests on win x86

72e0dec


          fix tests

de6897a


          Add a nuget.config with dotnet6 source

a35c9ce

.. to the project directory, because we are using it outside the repo
checkout.


          Add a nuget.config with dotnet6 source

c87673c

.. to the project directory, because we are using it outside the repo
checkout.


          Merge remote-tracking branch 'origin/main' into wasm-workloads-aot

3bfb526


          fix build

1b59652


          Fix test

8890ca3


          Fix tests on windows

2ca2d17

Member Author

radical commented Jun 29, 2021

@wli3 I wanted to check again if the E2E AOT tests here be acceptable? If not in this repo, then where should this be tested? We need to test the SDK, and the workload packs that are generated.

radical added 5 commits

July 2, 2021 13:00


          Merge remote-tracking branch 'origin/main' into wasm-workloads-aot

08f547c


          Merge remote-tracking branch 'origin/main' into wasm-workloads-aot

f59a358


          don't restrict PATH, because not sure which python3* binary would be …

d10c494

…needed


          actually, lets look for python3 since 3.6 seems to be the minimum

53f3940


          fix build

b3ad1c4

Member Author

radical commented Jul 8, 2021

Current state: Tests failing on linux because python is too old (<3.6).
Windows x86: workloads are not supported on win-x86, but not sure how to detect that, because RuntimeInformation.*Architecture return x64.


          Merge remote-tracking branch 'origin/main' into wasm-workloads-aot

413727d

radical force-pushed the wasm-workloads-aot branch from 50d9be1 to 413727d Compare

July 8, 2021 11:25

Member Author

radical commented Jul 19, 2021

Closing this, as the plan is to test these in a different repo.

radical closed this

radical deleted the wasm-workloads-aot branch

July 19, 2021 20:56

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet