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.

Sign up for GitHub

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

Jump to bottom

Add E2E tests for Pressable component #2

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add E2E tests for Pressable component #2

wants to merge 3 commits into from

Conversation

sansyrox
Copy link
Member

@sansyrox sansyrox commented Jun 10, 2020 •
edited
Loading

Summary

Fixes #1
Added E2E tests for the pressable component.

Changelog

[IOS] [Added] - Integration test for the pressable component

Test Plan

On running the following commands

detox build -c ios.sim.debug
detox test -c ios.sim.debug

The output is as shown below:

detox[49977] INFO:  [build.js] xcodebuild -workspace RNTester/RNTesterPods.xcworkspace -scheme RNTester -configuration Debug -sdk iphonesimulator -derivedDataPath RNTester/build -UseModernBuildSystem=NO -quiet
=== BUILD TARGET Flipper-RSocket OF PROJECT Pods WITH CONFIGURATION Debug ===
warning: no rule to process file '/Users/sakshijethi/Desktop/mlh-fellowship-react-native/react-native/RNTester/Pods/Flipper-RSocket/rsocket/benchmarks/CMakeLists.txt' of type text for architecture x86_64
warning: no rule to process file '/Users/sakshijethi/Desktop/mlh-fellowship-react-native/react-native/RNTester/Pods/Flipper-RSocket/rsocket/benchmarks/README.md' of type net.daringfireball.markdown for architecture x86_64
warning: no rule to process file '/Users/sakshijethi/Desktop/mlh-fellowship-react-native/react-native/RNTester/Pods/Flipper-RSocket/rsocket/README.md' of type net.daringfireball.markdown for architecture x86_64
detox[50032] INFO:  [test.js] configuration="ios.sim.debug" reportSpecs=true useCustomLogger=true DETOX_START_TIMESTAMP=1591795788991 node_modules/.bin/jest --config RNTester/e2e/config.json '--testNamePattern=^((?!:android:).)*$' --maxWorkers 1 "e2e"
detox[50033] INFO:  [DetoxServer.js] server listening on localhost:59180...
detox[50033] INFO:  [AppleSimUtils.js] com.facebook.react.uiapp launched. To watch simulator logs, run:
        /usr/bin/xcrun simctl spawn 822F3487-EBAD-4652-A225-6EE20D894605 log stream --level debug --style compact --predicate 'process == "RNTester"'
 PASS  RNTester/e2e/__tests__/TextInput-test.js (27.457 s)
  TextInput
    ✓ Live rewrite with spaces should replace spaces and enforce max length (3666 ms)
    ✓ Live rewrite with no spaces should remove spaces (3465 ms)
    ✓ Live rewrite with clear should remove spaces and clear (3928 ms)

detox[50033] INFO:  [DetoxServer.js] server listening on localhost:60251...
detox[50033] INFO:  [AppleSimUtils.js] com.facebook.react.uiapp launched. To watch simulator logs, run:
        /usr/bin/xcrun simctl spawn 822F3487-EBAD-4652-A225-6EE20D894605 log stream --level debug --style compact --predicate 'process == "RNTester"'
 PASS  RNTester/e2e/__tests__/DatePickerIOS-test.js (29.524 s)
  DatePickerIOS
    ✓ Should change indicator with datetime picker (4607 ms)
    ✓ Should change indicator with date-only picker (8504 ms)

detox[50033] INFO:  [DetoxServer.js] server listening on localhost:61102...
detox[50033] INFO:  [AppleSimUtils.js] com.facebook.react.uiapp launched. To watch simulator logs, run:
        /usr/bin/xcrun simctl spawn 822F3487-EBAD-4652-A225-6EE20D894605 log stream --level debug --style compact --predicate 'process == "RNTester"'
 PASS  RNTester/e2e/__tests__/Button-test.js (25.113 s)
  Button
    ✓ Simple button should be tappable (2159 ms)
    ✓ Adjusted color button should be tappable (2166 ms)
    ✓ Two buttons with JustifyContent:'space-between' should be tappable (3397 ms)
    ✓ Disabled button should not interact (646 ms)

detox[50033] INFO:  [DetoxServer.js] server listening on localhost:61313...
detox[50033] INFO:  [AppleSimUtils.js] com.facebook.react.uiapp launched. To watch simulator logs, run:
        /usr/bin/xcrun simctl spawn 822F3487-EBAD-4652-A225-6EE20D894605 log stream --level debug --style compact --predicate 'process == "RNTester"'
 PASS  RNTester/e2e/__tests__/Pressable-test.js (21.143 s)
  Pressable
    ✓ Console content should change on Press (1772 ms)
    ✓ Highlighted text should be pressable (1203 ms)
    ✓ Components with hitSlop prop extends touch area (1434 ms)

detox[50033] INFO:  [DetoxServer.js] server listening on localhost:61528...
detox[50033] INFO:  [AppleSimUtils.js] com.facebook.react.uiapp launched. To watch simulator logs, run:
        /usr/bin/xcrun simctl spawn 822F3487-EBAD-4652-A225-6EE20D894605 log stream --level debug --style compact --predicate 'process == "RNTester"'
 PASS  RNTester/e2e/__tests__/Touchable-test.js (36.597 s)
  Touchable
    ✓ Touchable Highlight should be tappable (2191 ms)
    ✓ Touchable Without Feedback should be tappable (1672 ms)
    ✓ Text should be tappable (1549 ms)

detox[50033] INFO:  [DetoxServer.js] server listening on localhost:62040...
detox[50033] INFO:  [AppleSimUtils.js] com.facebook.react.uiapp launched. To watch simulator logs, run:
        /usr/bin/xcrun simctl spawn 822F3487-EBAD-4652-A225-6EE20D894605 log stream --level debug --style compact --predicate 'process == "RNTester"'
 PASS  RNTester/e2e/__tests__/Picker-test.js (27.372 s)
  Picker
    ✓ should be selectable by ID (357 ms)

Test Suites: 6 passed, 6 total
Tests:       16 passed, 16 total
Snapshots:   0 total
Time:        167.355 s
Ran all test suites matching /e2e/i with tests matching "^((?!:android:).)*$".

@sansyrox sansyrox requested review from AnshG714 and jevakallio June 10, 2020 13:36
AnshG714
AnshG714 reviewed Jun 12, 2020
View reviewed changes
Copy link

@AnshG714 AnshG714 left a comment

Choose a reason for hiding this comment

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

I don't know if this is exactly needed, but it is possible to add more test cases. For example, the Pressable feedback events example could be tested out by simulating taps + long presses. Detox has a method for the latter too. Also, the disabled pressable can be tested by hooking it up to an alert of some kind (or any indication that the pressable has been pressed will do - this can be a text component as well).

@sansyrox
Copy link
Member Author

@AnshG714 , thanks! I'll have a look at the docs.

I don't know if this is exactly needed, but it is possible to add more test cases. For example, the Pressable feedback events example could be tested out by simulating taps + long presses.

This makes sense. I'll add it immediately! 🖖

Also, the disabled pressable can be tested by hooking it up to an alert of some kind (or any indication that the pressable has been pressed will do - this can be a text component as well).

jevakallio
jevakallio reviewed Jun 15, 2020
View reviewed changes
Copy link

@jevakallio jevakallio left a comment

Choose a reason for hiding this comment

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

@stealthanthrax the code looks great! Same test naming comments as on the other PR apply here, would like to see these conform to the it should... naming, and have the test descriptions focus on the RN Component under test, and not on the test screen elements

@sansyrox sansyrox requested a review from jevakallio June 16, 2020 22:20
jevakallio
jevakallio approved these changes Jun 17, 2020
View reviewed changes
Copy link

@jevakallio jevakallio left a comment

Choose a reason for hiding this comment

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

All good, one minor typo fix in suggestions

@sansyrox @jevakallio
Fix typo in Pressable-test
fd228a9 
Co-authored-by: Jani Eväkallio <[email protected]>
@jevakallio jevakallio added the pending merge This PR is ready to merge, but requires Mentor/PO action to merge upstream label Jun 18, 2020
pull bot pushed a commit that referenced this pull request Jul 1, 2020
@JoshuaGross @facebook-github-bot
Remove useless Mounting diagnostic error
0aa8ed6 
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AnshG714 AnshG714 AnshG714 left review comments

@jevakallio jevakallio jevakallio approved these changes

Assignees
No one assigned
Labels
pending merge This PR is ready to merge, but requires Mentor/PO action to merge upstream
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Write E2E tests for the Pressable component
3 participants
@sansyrox @jevakallio @AnshG714