Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade React Native to 0.64 #16562

Merged
merged 24 commits into from
Jun 16, 2021
Merged

Upgrade React Native to 0.64 #16562

merged 24 commits into from
Jun 16, 2021

Conversation

ceyhun
Copy link
Contributor

@ceyhun ceyhun commented May 21, 2021

Upgrading react-native to version 0.64.0

To test: Follow instructions on gutenberg-mobile PR: wordpress-mobile/gutenberg-mobile#3430

Regression Notes

  1. Potential unintended areas of impact
  • The whole Gutenberg editor could be affected
  1. What I did to test those areas of impact (or what existing automated tests I relied on)
  1. What automated tests I added (or what prevented me from doing so)
  • Gutenberg Mobile has it's own automated tests and doesn't rely on WPiOS's automated tests for now (except the limited EditorGutenbergTests)

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 21, 2021

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 21, 2021

You can trigger an installable build for these changes by visiting CircleCI here.

@cameronvoell cameronvoell added the Gutenberg Editing and display of Gutenberg blocks. label May 25, 2021
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 25, 2021

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

@ceyhun ceyhun marked this pull request as ready for review June 2, 2021 15:37
@ceyhun ceyhun added this to the 17.6 milestone Jun 2, 2021
@mokagio mokagio modified the milestones: 17.6, 17.7 Jun 14, 2021
@mokagio
Copy link
Contributor

mokagio commented Jun 14, 2021

Bumping this to 17.7 as part of the open PRs walkthrough before starting the 17.6 code freeze because there are conflicting files and it hasn't been reviewed yet.

Ping me here if this can't wait two weeks and needs to go into 17.6 once approved. 👍

@guarani
Copy link
Contributor

guarani commented Jun 16, 2021

Hi @ceyhun 👋

@buildkite buildkite/wordpress-ios — Build #629 failed (32 minutes, 53 seconds)

Looking at https://buildkite.com/wordpress-mobile/wordpress-ios/builds/629, the UI tests for iPhone and iPad failed with errors like:

[14:11:57]: ▸ EditorGutenbergTests
[14:12:33]: ▸ <span class="term-fg35">    ✗ testBasicPostPublish, XCTAssertTrue failed - Page WordPressUITests.PasswordScreen is not loaded.</span>

Is this a known issue?


A couple of other things I wasn't sure about:

  1. I went to the Gutenberg Mobile PR to check what to test and it says to try to run all the manual tests. I'm unsure if this means I should do the writing flow tests (as you did for Android here), or different manual tests (block tests for example)?
  2. Was the Regression Notes section left empty intentionally?

Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I smoke tested the app using the App Center build and didn't see any issues. I held off manual testing because I wasn't sure which areas need testing the most.

Podfile Show resolved Hide resolved
WordPress/WordPress.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
Podfile Show resolved Hide resolved
@ceyhun
Copy link
Contributor Author

ceyhun commented Jun 16, 2021

Thanks for the review @guarani!

Looking at buildkite.com/wordpress-mobile/wordpress-ios/builds/629, the UI tests for iPhone and iPad failed with errors like:
Is this a known issue?

I think it's safe to ignore Buildkite failures for now (internal references p1621980493008500-slack-CEJT26EG1 , p1623844140451700-slack-CC7L49W13).

  1. I went to the Gutenberg Mobile PR to check what to test and it says to try to run all the manual tests. I'm unsure if this means I should do the writing flow tests (as you did for Android here), or different manual tests (block tests for example)?

After that one, there was another pass of writing flow test both on WPiOS and WPAndroid mentioned here. That was done on a branch fixing WPAndroid errors targeting RN 0.64 upgrade branch. The changes for the fix were cherry-picked to the RN 0.64 upgrade branch.

I smoke tested the app using the App Center build and didn't see any issues. I held off manual testing because I wasn't sure which areas need testing the most.

Thanks again! I think that should be enough for now and other than that I guess it would take a lot of time to go through every manual test and we may have to rely on automated ones and the future release tests we'll have.

  1. Was the Regression Notes section left empty intentionally?

I thought the regression notes didn't really fit well with a big change like a React Native upgrade it's really hard to tell. I guess the whole Gutenberg editor can be affected by these changes and we didn't mostly rely on WPiOS's automated tests for catching any regressions. I'll add this in there.

@ceyhun ceyhun requested a review from guarani June 16, 2021 15:02
Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's safe to ignore Buildkite failures for now (internal references p1621980493008500-slack-CEJT26EG1 , p1623844140451700-slack-CC7L49W13).

Ah yes, I forgot that buildkite errors aren't blockers.

After that one, there was another pass of writing flow test both on WPiOS and WPAndroid mentioned here. That was done on a branch fixing WPAndroid errors targeting RN 0.64 upgrade branch. The changes for the fix were cherry-picked to the RN 0.64 upgrade branch.

Sounds good!

Thanks again! I think that should be enough for now and other than that I guess it would take a lot of time to go through every manual test and we may have to rely on automated ones and the future release tests we'll have.

I think that makes sense.

I thought the regression notes didn't really fit well with a big change like a React Native upgrade it's really hard to tell. I guess the whole Gutenberg editor can be affected by these changes and we didn't mostly rely on WPiOS's automated tests for catching any regressions. I'll add this in there.

Cool, makes sense. I just noticed you added "Gutenberg Mobile has it's own automated tests and doesn't rely on WPiOS's automated tests for now" above. My understanding was that EditorGutenbergTests is relied upon to some extent to catch editor regressions – even though it only really does a little bit of smoke testing of the editor. Not sure if that's what you meant, just wanted to mention it.

Looking forward to using 0.64! 🕺 Thanks for all the work here! 🙇

@ceyhun
Copy link
Contributor Author

ceyhun commented Jun 16, 2021

Cool, makes sense. I just noticed you added "Gutenberg Mobile has it's own automated tests and doesn't rely on WPiOS's automated tests for now" above. My understanding was that EditorGutenbergTests is relied upon to some extent to catch editor regressions – even though it only really does a little bit of smoke testing of the editor. Not sure if that's what you meant, just wanted to mention it.

I keep forgetting about that one 😄 But yeah, I guess we'll not be adding any more automated tests other than that for Gutenberg for the time being. Thanks a lot for the review 🙇

@ceyhun ceyhun enabled auto-merge June 16, 2021 19:01
@ceyhun ceyhun merged commit 95bd4c3 into develop Jun 16, 2021
@ceyhun ceyhun deleted the gutenberg/upgrade-RN-0-64 branch June 16, 2021 19:48
@mokagio
Copy link
Contributor

mokagio commented Jun 17, 2021

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gutenberg Editing and display of Gutenberg blocks.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants