Playwright: Run some real tests in Migration.E2E.Tests #32428
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
added
the
HaoK
commented
May 10, 2021
src/Components/test/E2ETestMigration/Microsoft.AspNetCore.Components.Migration.E2ETests.csproj
Show resolved
Hide resolved
HaoK
commented
May 10, 2021
src/Components/test/E2ETestMigration/Infrastructure/ServerFixtures/BlazorWasmTestAppFixture.cs
Show resolved
Hide resolved
HaoK
commented
May 12, 2021
HaoK
commented
May 12, 2021
HaoK
commented
May 12, 2021
|
|
||
| protected async Task ClearAndType(string selector, string value) | ||
| { | ||
| await _page.EvalOnSelectorAsync(selector, "e => e.Value = ''"); |
There was a problem hiding this comment.
todo: add extension static class and move helpers
HaoK
commented
May 12, 2021
HaoK
commented
May 12, 2021
| _tempDirectory = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N")); | ||
| Directory.CreateDirectory(_tempDirectory); | ||
|
|
||
| //Navigate(ServerPathBase, noReload: _serverFixture.ExecutionMode == ExecutionMode.Client); |
There was a problem hiding this comment.
E2E tests try to navigate/avoid reloads, revisit later (optimization?)
HaoK
commented
May 12, 2021
HaoK
commented
May 12, 2021
|
Might need to add some explicit ordering to the build pipelines, there's contention for writing the basictestapp now |
|
@pranavkm @captainsafia have you guys ever seen this error from publishing before? I've seen it happen a few times before, and now its happened on back to back builds, I copied the trimming target from E2E tests but made it unconditional for helix to get the published BasicTestApp for the payload. Any ideas what could cause this? Also got |
|
@javiercn do you want to start taking a look at this PR? Its got most of the initial shape done now minus some minor refactoring. We have all of the InputFiles test, and a few other tests migrated as some initial examples. Could probably move some more of the initialization boilerplate up somewhere, but I want to get your initial thoughts on things before I spend time cleaning up and polishing |
src/Components/test/E2ETestMigration/Microsoft.AspNetCore.Components.Migration.E2ETests.csproj
Outdated
Show resolved
Hide resolved
|
Still seeing build flakiness: |
|
Another potential variant of build ordering error in GenerateBlazorWebAssemblyBootJson: |
|
3rd variation of build contention |
src/Components/test/E2ETestMigration/Infrastructure/ServerTestBase.cs
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| //[Fact] | ||
| //public void CanBindTextbox_InitiallyPopulated() |
There was a problem hiding this comment.
Are you wanting to convert these tests before merging the PR or?
There was a problem hiding this comment.
No, I just want a small set of tests to start with, there's 12 right now. I don't want to onboard too many tests before we are happy and settle on the overall pattern. Its already a bit of a hassle to have to update so many tests when changing some of test patterns.
I had some open questions still about whether it was safe or a good idea to reuse the browser navigation page (TestPage property), I had to do some slightly sketchy stuff to make this work with the BrowserKind attribute.
I also wanted to make sure we were happy with the existing MountTestComponent pattern before going too much farther.
This is the first time I've seen how these test were setup, but now is a good time to consider if there's any improvements to make here. I don't know have anything in mind in terms of changes or improvements, but I wanted to double check we are happy with this method of test selection
There was a problem hiding this comment.
In terms of next steps, I think once we are mostly happy with the shape of things, I'll probably try turning on a single test in every file to play minesweeper and see what explodes and find where the gaps still are (in a future PR)
javiercn
reviewed
May 26, 2021
| return; | ||
| } | ||
|
|
||
| TestBrowser = await BrowserManager.GetBrowserInstance(browserKind, BrowserContextInfo); |
There was a problem hiding this comment.
Is this launching a new browser every time?
There was a problem hiding this comment.
Yes, I still have to get to #31111 which will tackle figuring out the best way to share fixtures/browser instances. Seems like we could just have a static BrowserManager instead of a shared fixture too right?
Update .azure/pipelines/ci.yml Co-authored-by: Safia Abdalla <[email protected]> Update helix-matrix.yml Update helix-matrix.yml Break test to verify browser/platform coverage Debug avail browsers Update Build.props Update ComponentTestBase.cs Don't delete temp directory if not set Update ComponentTestBase.cs Update playwrightSettings.linux.json Update playwrightSettings.ci.json Update BinaryHttpClientTest.cs Update BinaryHttpClientTest.cs Update BinaryHttpClientTest.cs Try turning off build in parallel Create copy of basic test app -> mega test app Update playwrightSettings.json Fix publish path Add new test project to slns, skip mariner Revert chromium change on ci.linux Reenable parallel builds Turn off parallel again Turn off test parallelization Update src/Components/test/testassets/MegaTestApp/MegaTestApp.csproj Co-authored-by: Safia Abdalla <[email protected]> Delete mega test app Update Microsoft.AspNetCore.Components.Migration.E2ETests.csproj Update AspNetCore.sln Update AspNetCore.sln Update Build.props Update Components.slnf Update BrowserTestBase.cs Update PlaywrightFixture.cs
javiercn
approved these changes
Aug 5, 2021
3 tasks
|
Huzzah! |
I just binged Mythic Quest last week, so I can't help but think of Everlight when I hear that :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes
Issues:
Stability stats after moving to separate test project:
0 / 1 runs no build issues
Build issues seen sometimes:
Microsoft.NET.Sdk.Razor\build\netstandard2.0\Microsoft.NET.Sdk.Razor.CodeGeneration.targets