[Templating][Components] Unify and improve E2E testing infrastructure #8188

javiercn · 2019-03-04T20:51:07Z

Unify the Templating and Components testing infrastructure.
Enable test project discovery in the components E2E tests.

javiercn · 2019-03-04T20:51:52Z

src/Components/test/E2ETest/Microsoft.AspNetCore.Components.E2ETests.csproj

@@ -1,10 +1,10 @@
 <Project Sdk="Microsoft.NET.Sdk">

+  <!-- Shared testing infrastructure for running E2E tests using selenium -->
+  <Import Project="$(SharedSourceRoot)E2ETesting\E2ETesting.props" />


This import and the targets import below is all that's needed to bring in Selenium E2E tests

ryanbrandenburg

These changes seem fine. You had talked about improving error detection and spiting tests into parts, I assume that's coming at a later date?

src/ProjectTemplates/test/MvcTemplateTest.cs

.azure/pipelines/jobs/default-build.yml

SteveSandersonMS · 2019-03-05T09:45:27Z

src/ProjectTemplates/test/RazorComponentsTemplateTest.cs

@@ -42,9 +47,12 @@ private void TestApplication(bool publish)

        private void TestBasicNavigation()
        {
+            // Give components.server enough time to load so that it can replace
+            // the prerendered content before we start making assertions.
+            Thread.Sleep(5000);


This seems a bit sketchy. It might be OK temporarily, but maybe we should extend the connected/disconnected APIs that @pranavkm was working on to provide a connection status value so it can be determined directly.

Well, ideally what I need is some API to know that components.server.js finished initializing so that I can do ((IJavasScriptExecutor)Browser).Execute("snippet") and determine it's done initializing.

You could register a circuitHandler and wait for onConnectionUp to complete.

That's a crazy amount of work for these tests, and wouldn't work for Blazor in the future. We should let people know when we are done initializing stuff by for example, simply saving the boot promise inside the window[Blazor] object.

src/Shared/E2ETesting/BrowserFixture.cs

src/Shared/E2ETesting/XUnitExtensions/XUnitTestCollectionRunnerWIthAssemblyFixture.cs

SteveSandersonMS

Looks great to me! I'd be interested to know about whether this can now run cross-platform.

javiercn · 2019-03-05T09:55:46Z

Looks great to me! I'd be interested to know about whether this can now run cross-platform.

I'm looking to see if I can make it run on OS X (and potentially linux)

pranavkm · 2019-03-05T16:59:27Z

src/ProjectTemplates/test/RazorComponentsTemplateTest.cs

@@ -42,9 +47,12 @@ private void TestApplication(bool publish)

        private void TestBasicNavigation()
        {
+            // Give components.server enough time to load so that it can replace
+            // the prerendered content before we start making assertions.
+            Thread.Sleep(5000);


You could register a circuitHandler and wait for onConnectionUp to complete.

src/Shared/E2ETesting/BrowserFixture.cs

pranavkm · 2019-03-05T17:35:14Z

src/Shared/E2ETesting/E2ETesting.targets

+
+    <!-- JAVA -->
+    <Message Importance="High" Text="Ensuring JAVA is available" />
+    <Exec Command="java -version" />


Can we capture the exit code and print a message that says what needs to be done resolve this?

Why not <Exec Command="${JAVA_HOME}/bin/java.exe -version"/> ?

Because this needs to run cross-platform and needs to be on the PATH for https://github.com/aspnet/AspNetCore/pull/8188/files?utf8=%E2%9C%93&diff=unified#diff-4035093707e8bfd6d9ba2a44912dce70R9 to work

dougbu · 2019-03-06T16:58:24Z

.azure/pipelines/jobs/default-build.yml

@@ -123,6 +123,10 @@ jobs:
  - ${{ if and(eq(parameters.installJdk, 'true'), eq(parameters.agentOs, 'Windows')) }}:
    - powershell: ./eng/scripts/InstallJdk.ps1 '11.0.1'
      displayName: Install JDK 11
+    - powershell: Write-Host "##vso[task.prependpath]$env:JAVA_HOME\bin"
+      displayName: Prepend JAVA bin folder to the PATH.


Don't change $env:Path globally. Java is only needed in a few places.

This is needed for https://github.com/aspnet/AspNetCore/pull/8188/files?utf8=%E2%9C%93&diff=unified#diff-4035093707e8bfd6d9ba2a44912dce70R9 to succeed when called as part of yarn install

Is there a reason this is super problematic? It's only building on the CI (so local builds aren't affected) and goes away once the build finishes.

It's not "super problematic" but it's a break from how the build currently works and broader than seemed necessary. If we go this route (rather than, say, creating a yarn.bat script), should clean up the Gradlew script and anything else that updates $env:Path for Java on Windows.

dougbu · 2019-03-06T17:17:15Z

src/Shared/E2ETesting/E2ETesting.targets

+
+    <!-- JAVA -->
+    <Message Importance="High" Text="Ensuring JAVA is available" />
+    <Exec Command="java -version" />


Why not <Exec Command="${JAVA_HOME}/bin/java.exe -version"/> ?

javiercn · 2019-03-07T19:23:40Z

@dougbu This is blocked on you.

The requirements have been explained. Can you please withdrawn your request for changes? We want to get this in ASAP as we haven't been running these tests regularly in a long time and has caused us issues.

dougbu · 2019-03-06T20:38:51Z

.azure/pipelines/jobs/default-build.yml

@@ -123,6 +123,10 @@ jobs:
  - ${{ if and(eq(parameters.installJdk, 'true'), eq(parameters.agentOs, 'Windows')) }}:
    - powershell: ./eng/scripts/InstallJdk.ps1 '11.0.1'
      displayName: Install JDK 11
+    - powershell: Write-Host "##vso[task.prependpath]$env:JAVA_HOME\bin"
+      displayName: Prepend JAVA bin folder to the PATH.


It's not "super problematic" but it's a break from how the build currently works and broader than seemed necessary. If we go this route (rather than, say, creating a yarn.bat script), should clean up the Gradlew script and anything else that updates $env:Path for Java on Windows.

dougbu · 2019-03-07T20:09:43Z

Note

should clean up the Gradlew script and anything else that updates $env:Path for Java on Windows

isn't urgent and doesn't need to be included in this PR. If you don't include it, please file an issue.

ryanbrandenburg · 2019-03-08T18:39:22Z

Any chance we could get this in today? I'd like to see if it fixes any of the the flakiness for SPA template tests.

SteveSandersonMS · 2019-03-08T18:45:50Z

Javier mentioned he preferred to merge this on Monday, so he could be around to watch for any possible flakiness.

* Unify the Templating and Components testing infrastructure. * Enable test project discovery in the components E2E tests.

javiercn · 2019-03-11T16:42:38Z

@aspnet/brt

I've seen this failing several times.

ryanbrandenburg · 2019-03-11T16:54:31Z

https://github.com/aspnet/AspNetCore-Internal/issues/1913

javiercn requested a review from SteveSandersonMS March 4, 2019 20:51

javiercn commented Mar 4, 2019

View reviewed changes

javiercn requested a review from ryanbrandenburg March 4, 2019 20:58

javiercn requested a review from dougbu as a code owner March 4, 2019 22:19

ryanbrandenburg approved these changes Mar 4, 2019

View reviewed changes

src/ProjectTemplates/test/MvcTemplateTest.cs Outdated Show resolved Hide resolved

javiercn force-pushed the javiercn/test-infrastructure-flakyness branch from 7462f4b to 47e0624 Compare March 4, 2019 23:27

dougbu suggested changes Mar 5, 2019

View reviewed changes

.azure/pipelines/jobs/default-build.yml Outdated Show resolved Hide resolved

Eilon added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Mar 5, 2019

javiercn force-pushed the javiercn/test-infrastructure-flakyness branch 2 times, most recently from c47ef93 to 6c96b8a Compare March 5, 2019 07:42

SteveSandersonMS reviewed Mar 5, 2019

View reviewed changes

src/Shared/E2ETesting/BrowserFixture.cs Outdated Show resolved Hide resolved

SteveSandersonMS reviewed Mar 5, 2019

View reviewed changes

src/Shared/E2ETesting/XUnitExtensions/XUnitTestCollectionRunnerWIthAssemblyFixture.cs Show resolved Hide resolved

SteveSandersonMS approved these changes Mar 5, 2019

View reviewed changes

pranavkm approved these changes Mar 5, 2019

View reviewed changes

javiercn force-pushed the javiercn/test-infrastructure-flakyness branch 2 times, most recently from c6a8453 to 79b4480 Compare March 6, 2019 15:10

dougbu reviewed Mar 6, 2019

View reviewed changes

dougbu mentioned this pull request Mar 6, 2019

[automated] Merge branch 'release/2.1' => 'release/2.2' #7920

Merged

javiercn force-pushed the javiercn/test-infrastructure-flakyness branch 4 times, most recently from f14fb59 to 3772540 Compare March 7, 2019 00:31

javiercn requested a review from dougbu March 7, 2019 09:38

dougbu approved these changes Mar 7, 2019

View reviewed changes

[Templating][Components] Unify and improve E2E testing infrastructure

c7770a5

* Unify the Templating and Components testing infrastructure. * Enable test project discovery in the components E2E tests.

javiercn force-pushed the javiercn/test-infrastructure-flakyness branch from 3772540 to c7770a5 Compare March 11, 2019 15:32

javiercn merged commit d7a9606 into master Mar 11, 2019

natemcmaster deleted the javiercn/test-infrastructure-flakyness branch May 3, 2019 06:24

[Templating][Components] Unify and improve E2E testing infrastructure #8188

[Templating][Components] Unify and improve E2E testing infrastructure #8188

Uh oh!

Conversation

javiercn commented Mar 4, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanbrandenburg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

SteveSandersonMS left a comment

Choose a reason for hiding this comment

Uh oh!

javiercn commented Mar 5, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

javiercn commented Mar 7, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dougbu commented Mar 7, 2019

Uh oh!

ryanbrandenburg commented Mar 8, 2019

Uh oh!

SteveSandersonMS commented Mar 8, 2019

Uh oh!

javiercn commented Mar 11, 2019

Uh oh!

ryanbrandenburg commented Mar 11, 2019

Uh oh!

Uh oh!