Skip to content

Conversation

@asklar
Copy link
Member

@asklar asklar commented Apr 14, 2020

Improve/refactor e2etest and TreeDumpLibrary in a number of ways:

  • most importantly is that we've had test break but not fail the PR or CI - this is because wdio which we use as a test harness, does not set its exit code on success/failure, which is needed for Azure Pipelines to know whether it should fail the build or not.
    image

  • Because we were blind to test failures, the outputs have drifted away from the original masters. I'm re-baselining them based on what we have today. Component owners should do a pass on the new masters to make sure nothing looks weird (e.g. ImageRTL still shows FlowDirection=LeftToRight). Filed Verify that the updated masters are correct  #4589 to track this.

  • changes the test app UI to provide links to the master and output files, and to diff the two. I wanted to launch vscode in diff mode for that but since we are a UWP app, the only way we can launch a program like that is to use a protocol handler, which vscode doesn't expose for diffing. Filed: Provide a protocol handler for diffing two files. For the time being, we'll copy the command line to launch vscode in diff mode for the two files.

  • Adds the ability for the tree dump library to output json which will allow us to write tests that validate the output from the javascript side

  • Adds logic for optionally output the name of elements. I plan to turn this and json output on soon but wanted to get a baseline PR/CI first.

  • e2etest was crashing because of a bug in Switch: (0.62) Switch Will Redbox When Clicked #4596 . Disabled that control in the test app for the time being.

  • Added logic to the pipeline to capture crash dumps when e2etest app crashes

  • Added checks to make sure we don't try to build/run the e2etest app in scenarios where it isn't really supported because the masters would not match (anything other than Release|x64 at 100% DPI). There are bugs tracking these limitations that we must fix before rtm: E2ETest failure: Padding doesn't work for Views on VS 2019, v142 tools, Release x64 #4122 and Scaling is wrong in RNW, possible Yoga issue #4619

Microsoft Reviewers: Open in CodeFlow

acoates-ms and others added 22 commits April 10, 2020 16:34
* Created new Microsoft.ReactNative.sln
* Update PR to build vnext/Microsoft.ReactNative.sln
* Add params to disable ReactUWP nuget pack during PR
* Move playground app to use Microsoft.ReactNative

* formatting

* Bunch of code review feedback

* Change files

* formatting

* Delete unneeded gitignore

* Minor removal of diff

* Minor changes to try to avoid having to update cert all the time

* Fix bundle command

* No need to build rntester seperately, the release build of playground will do it from msbuild

* Remove test property
* Disable .net native for managed unit tests

* Change files

* Enable managed unit tests in ci loop

* re-merge unit test tasks, can't run arm anyway

* Remove prefer 32bit from 64bit flavors

* Update mstest dependencies

* Managed tests are separate again

* Add comment

* Add test adapter path

* x86 debug again

* Update test adapter folder

* Try again

* remove frameworkuap10

* Switch to appxrecipie

* Try to switch to installed version of vstest

* Remove installer task
* Prototype mockable MessageQueueThread and JSExecutor

* Declare MockJSExecutor::CallFunctionFunctor

* Add missing using std::unique_ptr

* Use WebSocketModule::MethodId

* clang format

* Make WebSocketModule methods override instead of virtual.

* Allow WebSocketModule to use custom resource factory

* Rename *Impl to *Mock

* Move Instance mocks into separate source files

* Move Instance mocks into separate source files

* Change files

* clang format

* Define WebSocketModule::SetResourceFactory
-Surface msbuild errors (shows as undefined right now)
-downloading nuget with Invoke-WebRequest will usually take a long time because of the PS progress bar (which sadly gets updated for every byte), so I'm disabling the progress bar (it only takes a second to download when the progress bar is turned off)
retry once if nuget failed since if you cancel at just the right time, you could end up with a truncated nuget.exe and are wedged unless you know which file to delete. Learned that the hard way.
-factor out calling onto powershell functions
-we were not exposing whether the function failed or not
* Don't set readyState on connection error

* Change files
* Simplified C++ macros and improved their comments

* Change files
@asklar asklar requested a review from a team as a code owner April 14, 2020 04:45
asklar and others added 2 commits April 14, 2020 02:51
…-zero on success/failure respectively. This is important to break the CI/PR build on test failures, which we weren't doing until now.

As a result of not really looking at the tests, the masters have drifted away from their original values. We should investigate whether the values we output today are correct as a follow up.
@asklar asklar changed the title TreeDumpLibrary - output in Json Fix E2E Tests, add CI break on test failure, improve TreeDumpLibrary Apr 14, 2020
@jonthysell jonthysell requested review from a team and removed request for a team April 15, 2020 21:47
@asklar asklar merged commit 4c320cf into microsoft:master Apr 16, 2020
asklar added a commit to asklar/react-native-windows that referenced this pull request Apr 19, 2020
…icrosoft#4585)

Improve/refactor e2etest and TreeDumpLibrary in a number of ways:

most importantly is that we've had test break but not fail the PR or CI - this is because wdio which we use as a test harness, does not set its exit code on success/failure, which is needed for Azure Pipelines to know whether it should fail the build or not.
image

Because we were blind to test failures, the outputs have drifted away from the original masters. I'm re-baselining them based on what we have today. Component owners should do a pass on the new masters to make sure nothing looks weird (e.g. ImageRTL still shows FlowDirection=LeftToRight). Filed microsoft#4589 to track this.

changes the test app UI to provide links to the master and output files, and to diff the two. I wanted to launch vscode in diff mode for that but since we are a UWP app, the only way we can launch a program like that is to use a protocol handler, which vscode doesn't expose for diffing. Filed: Provide a protocol handler for diffing two files. For the time being, we'll copy the command line to launch vscode in diff mode for the two files.

Adds the ability for the tree dump library to output json which will allow us to write tests that validate the output from the javascript side

Adds logic for optionally output the name of elements. I plan to turn this and json output on soon but wanted to get a baseline PR/CI first.

e2etest was crashing because of a bug in Switch: microsoft#4596 . Disabled that control in the test app for the time being.

Added logic to the pipeline to capture crash dumps when e2etest app crashes

Added checks to make sure we don't try to build/run the e2etest app in scenarios where it isn't really supported because the masters would not match (anything other than Release|x64 at 100% DPI). There are bugs tracking these limitations that we must fix before rtm: microsoft#4122 and microsoft#4619
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make TreeDump output more easily consumable from js E2E have been failing since around 4/2-4/3 E2ETest failures need to fail the PR build

7 participants