Move windows test app to react-native-test-app #552

Saadnajmi · 2020-11-02T22:53:32Z

Platforms Impacted

Description of changes

We use react-native-test-app to handle the native code of our android, iOS, and macOS test apps, rather than us creating native projects for each and checking in the native code. As of 0.2.0, react-native-test-app added support for windows. Let's move move the windows test app to use react-native-test-app as well.

With this change, all platforms (except win32, which doesn't seem to have any native code anyway) now use react-native-test-app. Perhaps we could then combine the test apps back into apps/fluent-tester now?

Steps:

Add the react-native-test-app dependency to the windows test app package.json. Make sure it matches the version of the other test apps so that the node module can be hoisted. This also happened to alphabetize the package.json.
Remove everything under apps/windows/windows/ except for the .gitignore
Update app.json to have a appKey.
Run yarn install-windows-test-app to generate FluentTester.sln

Now, yarn windows should work as expected, meaning our CI should also work as expected.

This should close #447

Verification

Built the app on my windows PC.

Pull request checklist

This PR has considered (when applicable):

Saadnajmi · 2020-11-02T23:47:22Z

@tido64 Our CI to build the windows test app (fails with this change in a similar way that I see locally.
Specifically, running yarn windows --logging --release --arch x64 fails with an error that looks like this:

 Building Solution
- Build failed with message D:\a\1\s\apps\windows\windows\FluentTester.sln.metaproj : error MSB3202: The project file "D:\a\1\s\apps\windows\windows\..\node_modules\.generated\windows\ReactTestApp\ReactTestApp.vcxproj" was not found. [D:\a\1\s\apps\windows\windows\FluentTester.sln]. Check your build configuration.
✖ Build failed with message D:\a\1\s\apps\windows\windows\FluentTester.sln.metaproj : error MSB3202: The project file "D:\a\1\s\apps\windows\windows\..\node_modules\.generated\windows\ReactTestApp\ReactTestApp.vcxproj" was not found. [D:\a\1\s\apps\windows\windows\FluentTester.sln]. Check your build configuration.

Looks like either an incorrect node_modules path (probably bc monorepo?) or things are not getting generated into node_modules/.generated?

tido64 · 2020-11-03T00:25:43Z

Looks like either an incorrect node_modules path (probably bc monorepo?) or things are not getting generated into node_modules/.generated?

Did you add yarn install-windows-test-app to the CI build definition?

Saadnajmi · 2020-11-03T00:40:53Z

@

Looks like either an incorrect node_modules path (probably bc monorepo?) or things are not getting generated into node_modules/.generated?

Did you add yarn install-windows-test-app to the CI build definition?

Do I need to do that? I ran yarn install-windows-test-app once locally, and committed the generated .sln file. I thought all CI needs to do is run react-native run-windows and it should see no difference?

tido64 · 2020-11-03T00:44:26Z

Do I need to do that? I ran yarn install-windows-test-app once locally, and committed the generated .sln file. I thought all CI needs to do is run react-native run-windows and it should see no difference?

yarn install-windows-test-app generates the actual test app project under node_modules. The .sln file just references the project, similar to .xcworkspace referencing .xcproj.

Saadnajmi · 2020-11-03T00:48:53Z

I see, so this change now adds an extra step to run the windows test app kinda like how iOS/macOS need a pod install. Thanks, I'll update this PR and CI

tido64 · 2020-11-03T01:05:39Z

apps/windows/readme.md

+
+```
+    cd apps/windows
+    yarn install-windows-test-app


Could the yarn windows command also run install-windows-test-app?

I could... I could change the following in our package.json
"windows": "react-native run-windows" => "windows" : "install windows-test-app && react-native run-windows"

Just like how I could have done this for the iOS app
"ios": "react-native run-ios ..." => "ios": "pod install && react-native run-ios"

Buuuut something felt wrong about running two commands there to me. Not sure if that's mostly unfounded

Regardless, I could see that as a simplifying follow up change where I actually use that fluentui-scripts package we have to define tasks and run those.

@tido64 any idea why the CI over here is failing? I tried doing both scripts in yarn windows but that failed too

Comparing with RNTA, one difference I'm seeing is that we are specifying --sln to help the CLI find the solution. But here, the app builds so I don't think that's it. The other thing we can try is to not specify --release. I couldn't make that work locally due to some other issue:

$ react-native run-windows --sln windows/Example.sln --release √ Auto-linking... Success: No auto-linking changes necessary. (1404ms) √ Found MSBuild v16.0 at C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\MSBuild\Current\Bin (16.7.30523.141) √ Restoring NuGet packages √ Found Solution: D:\Source\react-native-test-app\example\windows\Example.sln i Build configuration: Release i Build platform: x86 × Building Solution: D:\Source\react-native-test-app\example\node_modules\react-native-windows\PropertySheets\Bu... × Build failed with message 9>Failed to construct transformer : error : EBUSY: resource busy or locked, open 'D:\Source\react-native-test-app\example\msbuild.ProjectImports.zip' [D:\Source\react-native-test-app\example\node_modules\.generated\windows\ReactTestApp\ReactTestApp.vcxproj]. Check your build configuration.

Okay, I found a workaround for the EBUSY error (in microsoft/react-native-windows-samples#174) and finally reproed the issue. We are outputting a .msix file but @react-native-windows/cli expects .appx (submitted a fix here microsoft/react-native-windows#6421). This happens if WindowsTargetPlatformMinVersion is high enough. Patching it locally, I can get passed the error but I'm hitting some issues with app signing. I can look into it when I have some time later...

You will also need this patch: microsoft/react-native-test-app#241. You'll need both this and the @react-native-windows/cli patch. I don't know if you can bump @react-native-windows/cli alone. If not, you'll have to look into patching it somehow.

I can probably wait till both react-native-test-app and react-native-windows bump, and update the dependencies and rerun the CI. In the meantime, I'll try to apply changes locally and see if I can find more issues

FYI, both PRs are merged. For the fixes in react-native-test-app, you should use 0.2.16. The fix in @react-native-windows/cli is not yet released. It will likely hit 0.64 if I understand the release process correctly.

…native into windows-test-app

apps/windows/windows/FluentTester/FluentTester.vcxproj

Saadnajmi · 2020-12-09T08:00:26Z

Just as an update, I'll return to this PR after ##566 is merged, where I also update to use the latest version of RNTA. (man that's a mouthful to type). I'm not sure if this is blocked on react-native-windows 0.64 getting released, but I'll investigate that when I return to this PR.

…nto windows-test-app

Saadnajmi · 2020-12-10T10:29:12Z

/azp run

azure-pipelines · 2020-12-10T10:29:29Z

Azure Pipelines successfully started running 1 pipeline(s).

…nto windows-test-app

Saadnajmi · 2021-03-04T21:01:30Z

Ok.. so the process name is "ReactTestApp" on the CI machine, but "FluentTester" on my local machine. Interesting.

tido64 · 2021-03-04T21:38:09Z

.ado/templates/e2e-testing-uwp.yml

+      Get-Process
+    displayName: 'Get List of processes'
+
+  # Kill ReactTestApp, leave server up and running. This was the only way I could get the server continuously running


In PowerShell, wouldn't one use something like Start-Process to fork a new process? Starting up the test app seems like a really expensive workaround…

I didn't write this, but I think the interntion was to test that the test app does run, and then do other stuff after

The app gets started, then is immediately killed afterwards. There is no time to test anything. The comment also says "This was the only way I could get the server continuously running".

Pinging @samuelfreiberg, since he wrote it.

Sorry for such a delayed response, not sure it matters anymore, but I couldn't get the server to run when simply starting it like normal. When using "yarn start" or whatever the command was, it wasn't starting the server. The only way I could get it to run at the time was like this. Running the test app also started the server, but then I had to kill the test app or the E2E tests wouldn't run properly. It was a 'hack' and maybe it's changed now, but it was very strange at the time.

tido64 · 2021-04-01T13:27:39Z

FYI, I'm adding more capabilities to React Native Test App:

Ability to use NuGet: fix(windows): restore ability to use NuGet react-native-test-app#301
Support for custom Package.appxmanifest: fix(windows): support for custom Package.appxmanifest react-native-test-app#302

…nto windows-test-app

Saadnajmi · 2021-04-01T17:05:52Z

FYI, I'm adding more capabilities to React Native Test App:

Ability to use NuGet: microsoft/react-native-test-app#301

Support for custom Package.appxmanifest: microsoft/react-native-test-app#302

I saw, thank you for that! Sorry I have not had the bandwidth to see this PR more as I don't have much bandwidth to fix windows CI errors.It also.... somehow didn't block our update to RN 0.63 which I thought we would need it to update our template app 🤷🏾

…nto windows-test-app

apps/windows/package.json

samuelfreiberg · 2021-06-25T16:11:09Z

I'm not too familiar with Metro and some of the other changes in this PR, I'll defer to someone else who might be able to validate these changes better. But most of it looks goof

…nto windows-test-app

yarn.lock

Saadnajmi · 2021-07-14T01:43:04Z

Closing because #755 exists and is replacing this PR.

Saadnajmi added 3 commits November 2, 2020 11:54

initial commit

82d056c

remove old windows test app

0053672

Update readme

9e8b502

Saadnajmi requested review from tido64, FalseLobster and HeyImChris November 2, 2020 22:53

Saadnajmi marked this pull request as draft November 2, 2020 23:18

Update depcheck

33ca1fe

Saadnajmi removed request for FalseLobster and HeyImChris November 2, 2020 23:43

Update CI and documentation with yarn install-windows-test-app

40158e9

tido64 reviewed Nov 3, 2020

View reviewed changes

Saadnajmi added 3 commits November 3, 2020 14:17

Merge branch 'master' of https://github.com/microsoft/fluentui-react-…

cf026c6

…native into windows-test-app

Edit yarn windows command

6c1a245

add generated sln to gitignore, update yml again

a7f701a

tido64 reviewed Nov 4, 2020

View reviewed changes

apps/windows/windows/FluentTester/FluentTester.vcxproj Show resolved Hide resolved

Saadnajmi mentioned this pull request Dec 4, 2020

Update react-native-test-app dependency, which allows RN 0.63 in macOS test app, and run-android CLI support #566

Merged

10 tasks

Saadnajmi added 2 commits December 10, 2020 04:16

Merge branch 'master' of github.com:microsoft/fluentui-react-native i…

14c275d

…nto windows-test-app

Update RNTA dependency, and add some missing scripts

2725e11

Saadnajmi added 2 commits December 10, 2020 15:16

Merge branch 'master' of github.com:microsoft/fluentui-react-native i…

f34373a

…nto windows-test-app

Merge branch 'master' of github.com:microsoft/fluentui-react-native i…

3802a0b

…nto windows-test-app

Saadnajmi mentioned this pull request Feb 3, 2021

Move most of repo to RN 0.63 #591

Closed

10 tasks

Change process name

6bdf568

tido64 reviewed Mar 4, 2021

View reviewed changes

Saadnajmi and others added 2 commits March 15, 2021 01:45

Merge branch 'master' into windows-test-app

351eceb

Update package.json

8f4e1a1

Merge branch 'master' of github.com:microsoft/fluentui-react-native i…

463ad8a

…nto windows-test-app

Try updating RNTA

decb1e7

Saadnajmi mentioned this pull request Apr 26, 2021

Bump react-native-test-app version #665

Closed

10 tasks

tido64 mentioned this pull request Jun 25, 2021

chore: bump @rnx-kit/cli and set up @rnx-kit/dep-check #744

Merged

10 tasks

Saadnajmi added 2 commits June 25, 2021 09:48

Merge branch 'master' of github.com:microsoft/fluentui-react-native i…

ef0c249

…nto windows-test-app

Bump RNTA and RNW

bdc361c

tido64 reviewed Jun 25, 2021

View reviewed changes

apps/windows/package.json Outdated Show resolved Hide resolved

Saadnajmi added 2 commits June 25, 2021 10:04

Use NuGet

0dd27a7

Run init-test-app again to generate required files

8a229f0

Saadnajmi added 6 commits June 30, 2021 11:19

Merge branch 'master' of github.com:microsoft/fluentui-react-native i…

c46f3fa

…nto windows-test-app

change app id in wdio.conf.js

1db812e

yarn lock

811982a

reconfigure test app

6f1086f

yarn lock

e3d1063

remove windows metro config

650f86b

tido64 reviewed Jun 30, 2021

View reviewed changes

yarn.lock Show resolved Hide resolved

Saadnajmi added 2 commits June 30, 2021 12:15

Update deps of fluent-tester

4c11a02

remove one more reference to rnw 0.62

5f48101

tido64 mentioned this pull request Jul 1, 2021

chore: use react-native-test-app for Windows #755

Merged

13 tasks

Saadnajmi closed this Jul 14, 2021

Saadnajmi deleted the windows-test-app branch August 26, 2021 16:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move windows test app to react-native-test-app #552

Move windows test app to react-native-test-app #552

Saadnajmi commented Nov 2, 2020 •

edited

Loading

Saadnajmi commented Nov 2, 2020

tido64 commented Nov 3, 2020

Saadnajmi commented Nov 3, 2020

tido64 commented Nov 3, 2020

Saadnajmi commented Nov 3, 2020

tido64 Nov 3, 2020

Saadnajmi Nov 3, 2020

Saadnajmi Nov 4, 2020

tido64 Nov 4, 2020 •

edited

Loading

tido64 Nov 4, 2020

tido64 Nov 4, 2020

Saadnajmi Nov 4, 2020

tido64 Nov 9, 2020

Saadnajmi commented Dec 9, 2020

Saadnajmi commented Dec 10, 2020

azure-pipelines bot commented Dec 10, 2020

Saadnajmi commented Mar 4, 2021

tido64 Mar 4, 2021

Saadnajmi Mar 5, 2021

tido64 Mar 5, 2021

samuelfreiberg Apr 1, 2021

tido64 commented Apr 1, 2021

Saadnajmi commented Apr 1, 2021

samuelfreiberg commented Jun 25, 2021

Saadnajmi commented Jul 14, 2021

Move windows test app to react-native-test-app #552

Move windows test app to react-native-test-app #552

Conversation

Saadnajmi commented Nov 2, 2020 • edited Loading

Platforms Impacted

Description of changes

Verification

Pull request checklist

Saadnajmi commented Nov 2, 2020

tido64 commented Nov 3, 2020

Saadnajmi commented Nov 3, 2020

tido64 commented Nov 3, 2020

Saadnajmi commented Nov 3, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tido64 Nov 4, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Saadnajmi commented Dec 9, 2020

Saadnajmi commented Dec 10, 2020

azure-pipelines bot commented Dec 10, 2020

Saadnajmi commented Mar 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tido64 commented Apr 1, 2021

Saadnajmi commented Apr 1, 2021

samuelfreiberg commented Jun 25, 2021

Saadnajmi commented Jul 14, 2021

Saadnajmi commented Nov 2, 2020 •

edited

Loading

tido64 Nov 4, 2020 •

edited

Loading