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

Only generate methods with supported types #238

Merged
merged 2 commits into from
Aug 22, 2017

Conversation

DanielMSchmidt
Copy link
Contributor

We currently generate code that isn't supported by the detox client.
This causes confusion amongst developers adopting the generated code (see #228).

@DanielMSchmidt DanielMSchmidt self-assigned this Aug 17, 2017
@DanielMSchmidt DanielMSchmidt force-pushed the generation/only-methods-with-supported-types branch 2 times, most recently from 52c3105 to d00b2cc Compare August 17, 2017 19:38
Copy link
Member

@rotemmiz rotemmiz left a comment

Choose a reason for hiding this comment

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

A few of the methods you removed use params which are actually primitives in disguise

@param duration The duration of the long press.

@return A GREYAction that performs a long press on an element.
*/static actionForLongPressWithDuration(duration) {
Copy link
Member

Choose a reason for hiding this comment

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

duration is CFTimeInterval === double, no need to kill it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great, I will add this to the list

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rotemmiz I double checked it, it is not included in the switch statement in method invocation Therefore we need to remove it for now. I will add this functionality while extending MethodInvocation in the same manner. That way the commits are sane and make sense 👍

@@ -196,60 +148,6 @@ starting from the given start points.
};
}

/*@return A GREYAction that scrolls to the given content @c edge of a scroll view.
*/static actionForScrollToContentEdge(edge) {
Copy link
Member

Choose a reason for hiding this comment

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

edge is GREYContentEdge , an ENUM, returns an NSString

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to sanitize these enums. As the sanitizer isn't build yet I excluded them. My next goal is to readd these functions


@return A GREYAction that scrolls to the given content @c edge of a scroll view with the scroll
action starting from the given start point.
*/static actionForScrollToContentEdgeXOriginStartPercentageYOriginStartPercentage(edge, xOriginStartPercentage, yOriginStartPercentage) {
Copy link
Member

Choose a reason for hiding this comment

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

edge is an NSInteger

Copy link
Member

Choose a reason for hiding this comment

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

Probably needs to be normalized like you did before

30 degrees).

@return A GREYAction that performs a fast pinch on the view in the specified @c direction.
*/static actionForPinchFastInDirectionWithAngle(pinchDirection, angle) {
Copy link
Member

Choose a reason for hiding this comment

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

direction is NSInteger

@param on The switch control state.

@return A GREYAction to toggle a UISwitch.
*/static actionForTurnSwitchOn(on) {
Copy link
Member

Choose a reason for hiding this comment

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

yea, we have problems with bools

@param column The UIPickerView column being set.
@param value The value to set the UIPickerView.

@return A GREYAction to set the value of a specified column of a UIPickerView.
Copy link
Member

Choose a reason for hiding this comment

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

why was this removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was 100% a mistake!

@param date The date to set the UIDatePicker.

@return A GREYAction that sets a given date/time on a UIDatePicker.
*/static actionForSetDate(date) {
Copy link
Member

Choose a reason for hiding this comment

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

:(

@param duration The duration of the long press.

@return A GREYAction that performs a long press on an element.
*/static actionForLongPressAtPointDuration(point, duration) {
Copy link
Member

Choose a reason for hiding this comment

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

same goes here

Copy link
Contributor Author

@DanielMSchmidt DanielMSchmidt left a comment

Choose a reason for hiding this comment

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

Still need to end generation end to end tests.

@DanielMSchmidt DanielMSchmidt force-pushed the generation/only-methods-with-supported-types branch from d00b2cc to e275e26 Compare August 22, 2017 18:25
@DanielMSchmidt DanielMSchmidt force-pushed the generation/only-methods-with-supported-types branch from e275e26 to 71b6b4d Compare August 22, 2017 18:59
@DanielMSchmidt
Copy link
Contributor Author

DanielMSchmidt commented Aug 22, 2017

Added the tests and verified the comments you made. These functions should mostly be unsupported and I would add them together with the change in the iOS mapping in MethodInvocation as we add new functionality. <3 I would merge this as soon as I see the tests green, we can still make changes afterwards 👍

@DanielMSchmidt DanielMSchmidt merged commit d88f00a into master Aug 22, 2017
@DanielMSchmidt DanielMSchmidt deleted the generation/only-methods-with-supported-types branch August 22, 2017 19:59
@wix wix locked and limited conversation to collaborators Jul 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants