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

feat(android): add device.tap() and device.longPress(). #4534

Closed
wants to merge 16 commits into from

Conversation

gayablau
Copy link
Contributor

@gayablau gayablau commented Jul 28, 2024

Description

  • This pull request addresses the issue described here: #<?>

In this pull request, I have …


For features/enhancements:

  • I have added/updated the relevant references in the documentation files.

For API changes:

  • I have made the necessary changes in the types index file.

@gayablau gayablau changed the title add tap android and basic test feat device.tap android Jul 28, 2024
@gayablau gayablau changed the title feat device.tap android feat device.tap() android Jul 28, 2024
@gayablau gayablau changed the title feat device.tap() android feat device.tap() device.longPress() android Aug 1, 2024
@gayablau gayablau marked this pull request as ready for review August 1, 2024 07:26
@asafkorem asafkorem changed the title feat device.tap() device.longPress() android feat(android): add device.tap() and device.longPress(). Aug 7, 2024
@asafkorem asafkorem requested review from gosha212 and removed request for noomorph and d4vidi August 7, 2024 05:06
@asafkorem
Copy link
Contributor

@gayablau please fix the lint errors and unit tests.

Copy link
Contributor

@asafkorem asafkorem left a comment

Choose a reason for hiding this comment

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

Great work! 👏🏼 🤩
We also need to document this (device.md).

Comment on lines 54 to 57
async tap(point) {
return '';
}

Copy link
Contributor

Choose a reason for hiding this comment

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

add a similar method for longPress(arg1, arg2) (in DeviceDriverBase.js).

Comment on lines 88 to 117
it(':android: tap on screen', async () => {
await device.launchApp({ newInstance: true });
await element(by.text('Device Tap')).tap();
await device.tap();
await expect(element(by.text('Screen Tapped'))).toBeVisible();
});

it(':android: tap on screen by coordinates', async () => {
await device.launchApp({ newInstance: true });
await element(by.text('Device Tap')).tap();
const point = {x: 200, y: 243}
await device.tap(point);
await expect(element(by.text('Button Tapped'))).toBeVisible();
});

it(':android: long press on screen by coordinates', async () => {
await device.launchApp({ newInstance: true });
await element(by.text('Device Tap')).tap();
const point = {x: 150, y: 300}
await device.longPress(point);
await expect(element(by.text('Screen Long Pressed'))).toBeVisible();
});

it(':android: long press on screen by coordinates with duration', async () => {
await device.launchApp({ newInstance: true });
await element(by.text('Device Tap')).tap();
const point = {x: 200, y: 190}
await device.longPress(point, 2000);
await expect(element(by.text('Screen Long Custom Duration Pressed'))).toBeVisible();
});
Copy link
Contributor

Choose a reason for hiding this comment

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

let's extract these to a new suite (e.g. 06.device-tap.test.js)

Copy link
Contributor

Choose a reason for hiding this comment

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

also add e2e tests for long-press with (1) duration only and (2) no params

Copy link
Contributor

Choose a reason for hiding this comment

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

I would also add more tests:

  • should long press on specified point (expect for specific area to be pressed when passing the point param)
  • should not succeed in long pressing with point outside the target area

Check how we added a similar tests on 03.actions.test.js.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same for device.tap(point)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

about the spetific point test these are tap on button by coordinates ignoring status bar and long press on button by coordinates with duration ignoring status bar. about failing on press outside the are these are long press on screen with duration without specifying coordinates and tap on screen without specifying coordinates
tell me if It is what you meant

Comment on lines 248 to 255
async tap(point) {
let x = point?.x || 100;
let y = point?.y || 100;
const call = EspressoDetoxApi.tap(x, y);
await this.invocationManager.execute(call);
}

async longPress(point, duration) {
Copy link
Contributor

Choose a reason for hiding this comment

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

please add the relevant unit tests (androiddriver.test.js)

@asafkorem asafkorem self-assigned this Aug 7, 2024
@asafkorem
Copy link
Contributor

Also let's add assertion when calling this API on iOS (raise unsupported message) - we should have that until adding the support on iOS.

Copy link
Contributor

@gosha212 gosha212 left a comment

Choose a reason for hiding this comment

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

Great Work!
Please fix the tests and the lint

@gayablau gayablau closed this Sep 7, 2024
@gayablau
Copy link
Contributor Author

gayablau commented Sep 7, 2024

united android and ios branches. fixes of review pushed to #4542

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 this pull request may close these issues.

3 participants