Skip to content

Conversation

@asklar
Copy link
Member

@asklar asklar commented May 9, 2020

I'm going to break up my RNTester visual test PR (#4699) into independent pieces because it's too massive otherwise.

This part deals with improving tree dump capabilities:

  • re-do some of the tree walk which could render invalid JSON in some cases due to no visitable children (and we had a trailing comma in the JSON which is a syntax error)
  • ignore certain xaml types like scrollbars
  • add the ability to ignore certain parts of the tree by marking them as "". This is useful for pages like DatePicker which will show the current date so we don't want to fail the treedump comparison because of a difference in the date displayed.
  • add debug output to be able to figure out differences between expected and actual tree dumps
  • assume that a collapsed element is the same as a non-existent element (since this can happen e.g. for ActivityIndicator)
  • include Text in the list of properties to capture and compare
  • filter out emojis and other surrogate pairs since JSON doesn't support them (they'd have to be escaped)

Fixes #4846
Fixes #4680
Fixes #4589

Microsoft Reviewers: Open in CodeFlow

@asklar asklar requested a review from a team as a code owner May 9, 2020 07:08
tudorms and others added 3 commits May 9, 2020 07:21
* Delay load ChakraCore.dll

* Change files

* clean up unneeded project references

* Overwrite property after React.Cpp.props gets included

* Change files

* change file

* Fix WinUI3

* Reverse the conditional logic

* Revert some unnecessary changes. Consolidate the preprocessor defines in the props

Co-authored-by: tudorm <[email protected]>
@NickGerleman
Copy link
Contributor

This unblocks #4680

asklar and others added 24 commits May 11, 2020 10:10
* Improve run_wdio to print out failed tests
* Fire onLoad event when a bitmap image is opeed

* Change files

* add imageFailed

* remove firing of onload events in the createImageBrush block so that load events don't fire twice.
…icrosoft#4786)

* Expose ability for apps to provide their own RedBox implementation

* Change files

* minor changes

* Code review feedback

* minor fix

* format fixes

* minor change

* minor fix

* minor fix

* minor fix
Bumps [fp-ts](https://github.com/gcanti/fp-ts) from 2.5.4 to 2.6.0.
- [Release notes](https://github.com/gcanti/fp-ts/releases)
- [Changelog](https://github.com/gcanti/fp-ts/blob/master/CHANGELOG.md)
- [Commits](gcanti/fp-ts@2.5.4...2.6.0)

Signed-off-by: dependabot-preview[bot] <[email protected]>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
* Fixed ReactContext copy/move semantic

* Change files

* Fixed formatting

* Test to show how to use a delegate in ReactPropertyBag
* create MS.ReactNative.IntegrationTests project

* RNHost activation succeeds

* update

* update

* Change files

* add JS function call test

* Change files

* fix formatting

* submit current state

* ReactNativeHost activation test succeeds

* Change files

* account for property bag addition

* sync, address code review feedback
* Use spec file for DevSettings NativeModule

* Change files

* minor change
* Handle HTTP connect exceptions in DevSupportManager

* Change files
* Allow storing non-WinRT types in ReactPropertyBag

* Change files

* Addressed PR feedback
…icrosoft#4877)

* Use DispatcherQueue instead of CoreDispatcher for Mso::DispatchQueue

* Change files

* Removed 'Thread' from some new API names.

* Removed extra 'private' section in AppearanceModule

* Temporary: use CoreDispatcher to check the E2E test

* Avoid using DispatcherQueue::HasThreadAccess in earlier Windows versions.
* add rnw-dependencies.ps1

* Change files

* --sln

* run in CI

* print

* desktop appium is optional

* use agent directory

* 15GB is a ballpark estimate

* optional space

* .
…osoft#4893)

* Change files

* revert dfc57fc

* set default tab index to -1, the hope is to be in line with xbox assumption

* xbox doesn't seem to rely on this, changing default back to match xaml default
* add instructions for CI debugging for e2e test app

* point e2e readme to e2e testing doc
* don't exit the powershell session and pause for keyboard input if interactive

* Change files
@asklar asklar added the AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity) label May 14, 2020
@ghost
Copy link

ghost commented May 14, 2020

Hello @asklar!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@vmoroz
Copy link
Member

vmoroz commented May 14, 2020

@asklar, the change shows too many files changed. I guess it is mostly due to the merges that come from the master branch. I am signing off to get you unblocked because I am sure that you did your best to implement this change, but I could not understand what is part of the PR and what is not and did not really reviewed the PR. Please consider in future techniques that reduce the set of changed files in PR only to your changes.

I do not know what these techniques are and I wish to learn them too. Last time when I got into such situation, I closed current PR, created new branch where I squash committed all my changes and sent another PR from that branch. It was showing only one commit and only changes that I did. I am sure there is a better approach, but as a rule of thumb now I always create new branch and do a squash commit there before I use it for a new PR, and it works.

@ghost ghost merged commit c393438 into microsoft:master May 14, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity)

Projects

None yet

10 participants