Selenium tests no longer run locally in VS

[SteveSandersonMS]

Maybe I missed a memo about something we're meant to do differently now, but I think this problem was introduced here: f76be64#diff-d25609f62dd6bb91ad82d484d1f32bfc

Since that commit, local builds in VS no longer write the necessary assembly metadata to enable the Selenium tests, so you can't run and debug them from the IDE. We definitely need to be able to do that.

@JunTaoLuo Is there a new and intended way to run those tests from VS? Or am I just doing something wrong?

[JunTaoLuo]

The motivation for the original issue we were trying to solve (#10504) is that building the repo doesn't require Java and should not need to specify -NoBuildJava. I can look into potential implementations where builds don't check for Java installations but tests will still fail without Java. In the meantime, the expected workflow we had in mind is that you would set BuildJava as an environment variable or passed in via command line arguments. In the meantime, you should be able to resolve the issue by launching via a shell using startvs.cmd with the environment variable set.

[JunTaoLuo]

cc @mkArtakMSFT

[SteveSandersonMS]

I don’t think it’s ok to have the requirement that people reverse-engineer our build system to figure out what magic environment variables to pass to VS just so they can run tests.

It’s not just about engineers at Microsoft - this is meant to be an open source project, which implies external developers should be able to make sense of how to contribute.

[JunTaoLuo]

The alternative is that ./build.cmd will fail for everyone working in this repo, even for people working in areas that don't run Seleium tests.

Again, I'm not proposing a permanent workflow change, but to unblock, see if setting the environment variable would resolve your issue. I agree that we shouldn't require external developers reverse engineering our builds, but we'll need to investigate possible improvements.

Some alternatives that come to mind include add a warning that selenium tests are not running and instructions on how to enable them, or run tests automatically if Java is detected but skip them if Java is not found.

[mkArtakMSFT]

@JunTaoLuo I thought we agreed earlier to default the flag to a value, which would result simple ./build.cmd to succeed. Are you saying that's the goal, but it's not straight forward, so for now we'll need to have a workaround?

[JunTaoLuo]

I realized that the solution we were considering probably won't be sufficient. Modifying https://github.com/aspnet/AspNetCore/blob/master/src/Components/build.cmd isn't enough because the actual issue here is test not build. We can run tests in two ways: command line and in VS. If we edited build.cmd and tell devs to test via ./build.cmd -Test, that would fix the command line test workflow but I don't think that's what most people are doing. Also, changing the behaviour of build.cmd will not affect how VS runs tests which is also a requirement here. Hence, I suggested the workaround of setting the environment variable for now while we investigate if there's a better way to implement this. I have made two proposals but open to any other ideas.

[rynowak]

Hence, I suggested the workaround of setting the environment variable for now while we investigate if there's a better way to implement this.

I'm still blocked on this. Setting the environment variable is not enough to make this work.

System.ArgumentNullException: Value cannot be null.
Parameter name: path2
   at System.IO.Path.Combine(String path1, String path2)
   at Microsoft.AspNetCore.Blazor.DevServer.Server.Startup.ResolveApplicationAssemblyFullPath(IWebHostEnvironment environment) in C:\git\aspnet\AspNetCore\src\Components\Blazor\DevServer\src\Server\Startup.cs:line 59
   at Microsoft.AspNetCore.Blazor.DevServer.Server.Startup.Configure(IApplicationBuilder app, IWebHostEnvironment environment, IConfiguration configuration) in C:\git\aspnet\AspNetCore\src\Components\Blazor\DevServer\src\Server\Startup.cs:line 38
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at Microsoft.AspNetCore.Hosting.MethodInfoExtensions.InvokeWithoutWrappingExceptions(MethodInfo methodInfo, Object obj, Object[] parameters) in C:\git\aspnet\AspNetCore\src\Hosting\Hosting\src\Internal\MethodInfoExtensions.cs:line 17
   at Microsoft.AspNetCore.Hosting.ConfigureBuilder.Invoke(Object instance, IApplicationBuilder builder) in C:\git\aspnet\AspNetCore\src\Hosting\Hosting\src\Internal\ConfigureBuilder.cs:line 56
   at Microsoft.AspNetCore.Hosting.ConfigureBuilder.<>c__DisplayClass4_0.<Build>b__0(IApplicationBuilder builder) in C:\git\aspnet\AspNetCore\src\Hosting\Hosting\src\Internal\ConfigureBuilder.cs:line 20
   at Microsoft.AspNetCore.Hosting.ConventionBasedStartup.Configure(IApplicationBuilder app) in C:\git\aspnet\AspNetCore\src\Hosting\Hosting\src\Startup\ConventionBasedStartup.cs:line 25
   at Microsoft.AspNetCore.HostFilteringStartupFilter.<>c__DisplayClass0_0.<Configure>b__0(IApplicationBuilder app) in C:\git\aspnet\AspNetCore\src\DefaultBuilder\src\HostFilteringStartupFilter.cs:line 17
   at Microsoft.AspNetCore.Hosting.WebHost.BuildApplication() in C:\git\aspnet\AspNetCore\src\Hosting\Hosting\src\Internal\WebHost.cs:line 228

[mkArtakMSFT]

@JunTaoLuo, @dougbu this is still blocking the team. I'm marking this as P0 so it gets enough attention.

[dougbu]

@mkArtakMSFT the logic to detect whether Java is installed (locally or otherwise) is in Bash and PowerShell scripts at the moment, not MSBuild files. That makes detection when running tests in VS a more time-consuming change and I hesitate to prioritize the change this week. Is there a lesser / quicker suggestion that's enough for now?

Separately, it's not clear how @rynowak's stack trace relates to enabling Selenium tests. The basic requirements are (a) have Java installed and on $env:Path (satisfied using eng/scripts/InstallJdk.ps1 for example) and (b) pass /p:BuildJava=true into msbuild or set the equivalent environment variable. If building Java isn't actually necessary for these tests, may be fine to use $(SeleniumE2ETestsSupported) or $env:SeleniumE2ETestsSupported instead of the BuildJava equivalents.

[mkArtakMSFT]

Adding @pranavkm in case he has some ideas regarding this.

[SteveSandersonMS]

I'm still blocked on this. Setting the environment variable is not enough to make this work.

The way I got the E2E tests running in VS was to modify Shared/E2ETesting/E2ETesting.props, changing the SeleniumE2ETestsSupported condition to:

<SeleniumE2ETestsSupported Condition="'$(SeleniumE2ETestsSupported)' == '' and '$(TargetArchitecture)' != 'arm' and '$(OS)' == 'Windows_NT'">true</SeleniumE2ETestsSupported>

(i.e., removing the and '$(BuildJava)' == 'true' clause)

Obviously I'm not suggesting this is a good solution, just a temporary way to be unblocked.

[javiercn]

Why is BuildJava required in the first place, I might have overlooked the changes when I signed off on them, but if the build doesn't fail without it, there's no reason for it to be there, it only gets in the way.

[dougbu]

@javiercn BuildJava is an indication that Java is available (build startup fails if user runs ./build.ps1 -buildJava or ./build.sh --build-java and Java is not found). The projects don't otherwise have a way to decide whether to attempt the Selenium tests.

Basically, this is all about allowing users to not have Java installed if they're working in another part of the repo.

[dougbu]

@SteveSandersonMS

changing the SeleniumE2ETestsSupported condition

Do the tests also work for you if you define $env:SeleniumE2ETestsSupported = true and start VS from the command line where you did that? The condition you changed should just provide a default value.

[JunTaoLuo]

The original issue shows the error anyone without java runs into:

Build FAILED.

D:\code\AspNetCore\src\Shared\E2ETesting\E2ETesting.targets(41,5): error MSB3073: The command "java -version" exited with code 9009. [D:\code\AspNetCore\src\Components\test\E2ETest\Microsoft.AspNetCore.Components.E2ETests.csproj]

    0 Warning(s)
    2 Error(s)

This shows up when you run ./build.cmd in the root if you don't have java installed.

[javiercn]

@JunTaoLuo There is an explicit MSBuild variable to prevent that from happening $(EnforcePrerequisites). Could you setup that to false in build.cmd?

[javiercn]

https://github.com/aspnet/AspNetCore/blob/master/src/Shared/E2ETesting/E2ETesting.targets#L34

[javiercn]

https://github.com/aspnet/AspNetCore/blob/master/src/Shared/E2ETesting/E2ETesting.props#L7

[javiercn]

The outcome that I expect for this is that it EnforcePrerequisites is disabled if you do build.cmd from a developer machine and is enabled in all other cases.

• If you are building the components repo.
• If you are building the templates repo.
• If you are builiding the project templates solution from visual studio.
• If you are building the components solution from visual studio.

[javiercn]

This change has not only affected the developer experience but also has changed the behavior of some tests. As it has silently disabled running the automation for the E2E tests. As this PR proofs #11602

Given this is the case, we should remove the BuildJava flag and use the EnforcePrerequisites variable instead as described above.

I have a change out to remove the check for BuildJava #11603 and will merge once the PR passes, as this has removed coverage from all our E2E tests and that is not acceptable.

[JunTaoLuo]

Yes that should work.

[JunTaoLuo]

Thanks @javiercn !

[javiercn]

This is not completely closed. I had to skip the component tests because they hang on the CI

[dougbu]

@javiercn or @JunTaoLuo does #12030 track the remaining work in this area? If yes, please close this one as resolved. If not, please make it clear what will be needed that isn't covered in #12030.

[JunTaoLuo]

Actually the tracking issue is #11698.

[javiercn]

I think this was fixed as part of 1480b99

E2E test for components still don't run on the CI