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

Write E2E tests for the Pressable component #1

Open
sansyrox opened this issue Jun 9, 2020 · 0 comments · May be fixed by #2
Open

Write E2E tests for the Pressable component #1

sansyrox opened this issue Jun 9, 2020 · 0 comments · May be fixed by #2

Comments

@sansyrox
Copy link
Member

sansyrox commented Jun 9, 2020

Please provide all the information requested. Issues that do not follow this format are likely to stall.

Description

To write Integration tests for the Pressable component in the RNTester App.

Expected Results

To learn how to write tests in accordance with flow syntax and a list of tests which work with the Pressable component in the App.

Snack, code example, screenshot, or link to a repository:

Screenshot 2020-06-10 at 3 01 22 AM

@sansyrox sansyrox linked a pull request Jun 10, 2020 that will close this issue
pull bot pushed a commit that referenced this issue Jul 1, 2020
Summary:
While in theory we should never delete views before removing them from the hierarchy, there are some exceptions:

(1) Some mysterious cases that don't seem like bugs, but where the child still seems to keep a reference to the parent:
(2) When deleting views as part of stopSurface.

On #1: in the past we had issues when we assumed that ViewManager.getChildCount() would return an accurate count. Sometimes it's just... wrong. Here, I've found at least one case where a View still has a parent after it's removed from the View hierarchy. I assume this is undocumented Android behavior or an Android bug, but either way, there's nothing I can do about it.

On #2: there are valid cases where we want to delete a View without explicitly removing it from the View hierarchy (it will eventually be removed from the hierarchy when the Root view is unmounted, but it may still be "in" a View hierarchy when it's deleted).

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D22321374

fbshipit-source-id: 9667bbe778c418f0216550638dc26ca48a58e5fa
pull bot pushed a commit that referenced this issue Oct 16, 2020
Summary:
changelog: [internal]

Prevents 2 type converions:
1. int <-> size_t
2. int <-> int32_t

# Why is using size_t better when working with indexes.

## 1. Type conversion isn't for free.

Take this example

```
size_t calculate(int number) {
  return number + 1;
}
```

It generates following assembly (generated with armv8-a clang 10.0.0):

```
calculate(int):                          // calculate(int)
sub     sp, sp, #16                     // =16
str     w0, [sp, #12]
ldr     w8, [sp, #12]
add     w9, w8, #1                      // =1
mov     w8, w9
sxtw    x0, w8
add     sp, sp, #16                     // =16
ret
```

That's 9 instructions.

If we get rid of type conversion:

```
size_t calculate(size_t number) {
  return number + 1;
}
```

Assembly (generated with armv8-a clang 10.0.0):

```
calculate(unsigned long):                          // calculate(unsigned long)
sub     sp, sp, #16             // =16
str     x0, [sp, #8]
ldr     x8, [sp, #8]
add     x0, x8, #1              // =1
add     sp, sp, #16             // =16
ret
```

Compiler now produces only 7 instructions.

## Semantics

When using int for indexing, the type doesn't say much. By using `size_t`, just by looking at the type, it gives the reader more information about where it is coming from.

Reviewed By: JoshuaGross

Differential Revision: D24332248

fbshipit-source-id: 87ef982829ec14906ed9e002ea2e875fda4a0cd8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant