Skip to content

e2e: use google_apis device #488

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

Merged
merged 16 commits into from
Jul 10, 2024
Merged

e2e: use google_apis device #488

merged 16 commits into from
Jul 10, 2024

Conversation

kirillzyusko
Copy link
Owner

@kirillzyusko kirillzyusko commented Jul 2, 2024

📜 Description

Enable tests on real Android image (not AOSP).

💡 Motivation and Context

The motivation is to execute tests on devices that are close to a real one. AOSP has its own advantages, such as stable tests. However one year later (since I added e2e tests support in GitHub) GitHub has upgraded its runners and now we can run real Android images and it opens a new opportunities:

  • we can test interactive keyboard;
  • potentially we can add tests that will change a type of the keyboard (emoji).
  • running a real image can assure we don't have bugs on real devcies (not AOSP only).

But this addition comes with own price - existing e2e tests will fail, because of asynchronous nature of "input focused" and "onApplyWindowInsets" events. In not AOSP these events are asynchronous, and current KeyboardAwareScrollView works in the next way:

  • if focus was changed - be sure it's visible and perform animated scroll;
  • if keyboard was resized - check input is still visible if not perform animated scroll.

And since these events are asynchronous - we can end up in various states:

  • we can scroll till the end, keyboard size becomes smaller and we don't scroll;
  • keyboard size becomes smaller and we just started to scroll -> in this case we'll strictly match 50px bottomOffset constraint;

And because of that it's really hard to cover such functionality by e2e tests. In this PR I added SOFT_CHECK variable - if it's true, then we perform checks via matchers (since we can not assure pixel-perfect precision).

📢 Changelog

E2E

  • run tests on Google APIs image;

🤔 How Has This Been Tested?

Tested on CI.

📸 Screenshots (if appropriate):

See in files changed 👀

📝 Checklist

  • CI successfully passed
  • I added new mocks and corresponding unit-tests if library API was changed

Sorry, something went wrong.

@kirillzyusko kirillzyusko added 🤖 android Android specific e2e Anything about E2E tests labels Jul 2, 2024
@kirillzyusko kirillzyusko self-assigned this Jul 2, 2024
Copy link
Contributor

github-actions bot commented Jul 2, 2024

📊 Package size report

Current size Target Size Difference
144411 bytes 144411 bytes 0 bytes 📉

kirillzyusko added a commit that referenced this pull request Jul 4, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
## 📜 Description

Fixed an issue where `scrollPosition.value` is updated with out-of-date
value.

## 💡 Motivation and Context

<!-- Why is this change required? What problem does it solve? -->
<!-- If it fixes an open issue, please link to the issue here. -->

The problem happens, because without animation `onMove`/`onEnd`
dispatched almost simultaneously. But js handler can not update
`position.value` and in `onEnd` handler in `scrollPosition.value =
position.value;` code we set `scrollPosition.value = 0`.

When text input grows:

```tsx
layout.value = input.value;
scrollPosition.value += maybeScroll(keyboardHeight.value, true);
```

We call `maybeScroll`, but `scrollPosition.value === 0` and because of
that we scroll by 16px (though we had to scroll for `actualScroll (170)
+ 16` but we do `0 + 16`, and because of that position is incorrect.

To fix the problem `onEnd` should actually update `scrollPosition` to
the expected value, and to achieve that we need to update a value from
worklet.

Now it's slightly problematic, because current REA doesn't support both:
js + worklet handlers (see
software-mansion/react-native-reanimated#6204),
so I'm always re-dispatching `nativeEvent` manually via `runOnJS`.

The issue was caught originally in
#488

## 📢 Changelog

<!-- High level overview of important changes -->
<!-- For example: fixed status bar manipulation; added new types
declarations; -->
<!-- If your changes don't affect one of platform/language below - then
remove this platform/language -->

### JS

- fix race condition between scroll position update and keyboard
movement.

## 🤔 How Has This Been Tested?

- Set focus to `TextInput#3`
- press enter 2 times

Tested on Android 12 (Pixel 2) with disabled animations (detox).

## 📸 Screenshots (if appropriate):

|Before|After|
|-------|-----|
|<img width="399" alt="image"
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/5511fb3e-54c3-4495-9955-13e933839941">|<img
width="397" alt="image"
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/00d679ba-5986-4fb6-a41a-9c6a1944c98d">|

## 📝 Checklist

- [x] CI successfully passed
- [x] I added new mocks and corresponding unit-tests if library API was
changed
@kirillzyusko kirillzyusko force-pushed the feat/use-google-apis-device branch 2 times, most recently from 8bfa473 to f0bf177 Compare July 10, 2024 09:41
@kirillzyusko kirillzyusko force-pushed the feat/use-google-apis-device branch from 7f603f9 to beb37c3 Compare July 10, 2024 10:59
@kirillzyusko kirillzyusko marked this pull request as ready for review July 10, 2024 14:35
@kirillzyusko kirillzyusko merged commit 4b6a06d into main Jul 10, 2024
10 checks passed
@kirillzyusko kirillzyusko deleted the feat/use-google-apis-device branch July 10, 2024 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 android Android specific e2e Anything about E2E tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant