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

Fix DatePickerIOS e2e tests #23861

Closed
wants to merge 3 commits into from
Closed

Fix DatePickerIOS e2e tests #23861

wants to merge 3 commits into from

Conversation

grabbou
Copy link
Contributor

@grabbou grabbou commented Mar 12, 2019 •
edited

Summary

DatePickerIOS tests stopped working after US changed their time zone on CircleCI. I decided to update the tests to be more reliable, by:

  • removing "timezoneoffset" from the examples - I don't think it's needed. We don't demonstrate other props and it was causing us some troubles when timezones actually changed
  • changing "interval" example from "time-only" to "datetime" because there's a known bug that prevented the previous example from working [DatePickerIOS] minuteInterval not work with mode="time" minimumDate={min} maximumDate={max} #9566
  • splitting the label to two: one for date and other one for time, so that we can match the date only when we test "date" mode only

Changelog

[IOS] [FIXED] - Improved reliability of DatePickerIOS e2e tests

Test Plan

Tests pass on CircleCI (see 0.59-stable)

kelset
kelset approved these changes Mar 12, 2019
View reviewed changes
Copy link
Collaborator

@kelset kelset left a comment

Choose a reason for hiding this comment

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

:shipit:

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 12, 2019
@grabbou
Copy link
Contributor Author

grabbou commented Mar 12, 2019

Detox tests are green, so this is ready to ship!

@ericlewis
Copy link
Contributor

Woot!

@grabbou grabbou requested a review from cpojer March 12, 2019 21:40
cpojer
cpojer approved these changes Mar 13, 2019
View reviewed changes
Copy link
Contributor

@cpojer cpojer left a comment

Choose a reason for hiding this comment

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

Timezones, how do they work.

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Mar 13, 2019
facebook-github-bot
facebook-github-bot approved these changes Mar 13, 2019
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @grabbou in 8270de9.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added Merged This PR has been merged. and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Mar 13, 2019
@zhongwuzw zhongwuzw mentioned this pull request Mar 14, 2019
Closed
facebook-github-bot pushed a commit that referenced this pull request Mar 15, 2019
@zhongwuzw @facebook-github-bot
Fixed minuteInterval invalid in time mode (#23923)
942bf40 
Summary:
From #23861 (comment)
> changing "interval" example from "time-only" to "datetime" because there's a known bug that prevented the previous example from working

We need to ensure set minuteInterval after set datePickerMode, otherwise minuteInterval invalid in time mode.

cc. grabbou cpojer .

[iOS] [Fixed] - Fixed minuteInterval invalid in time mode
Pull Request resolved: #23923

Differential Revision: D14477549

Pulled By: cpojer

fbshipit-source-id: 2c612d488b6d592b1907e150df5e07fe83132829
@hramos hramos deleted the fix/date-picker-e2e branch February 25, 2020 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cpojer cpojer cpojer approved these changes

@facebook-github-bot facebook-github-bot facebook-github-bot approved these changes

@kelset kelset kelset approved these changes

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Component: PickerIOS Merged This PR has been merged. Platform: iOS iOS applications.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@grabbou @ericlewis @react-native-bot @cpojer @facebook-github-bot @kelset