Bug #12094 - Fix _appiumStart method by lararojasmr · Pull Request #12095 · webdriverio/webdriverio

lararojasmr · 2024-01-26T02:22:26Z

Proposed changes

I've added a timeout to the method and fix the issue that happened when the promise emit data but is not include the information that is in the conditional, that could be a error.

I added a rejectOnce method to ensure that the reject is called once.

The bug is #12094.

Types of changes

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update

Checklist

I have read the CONTRIBUTING doc
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)
I have added proper type definitions for new commands (if appropriate)

Further comments

Reviewers: @webdriverio/project-committers

…smine. Update the Cucumber Step Mode. Change constants and option names.

- using 'getErrorFromFailedTest' to format error messages

- removing unnecessary code for mocha and jasmine

- eslint and spaces before from.

- Property 'labels' does not exist on type 'ExecutableItem'. - error TS2835: Relative import paths need explicit

- Allure Report Methods. - Eslint Fixes - Test Cases wdio-allure-reporter related

- Fix undefined in fixtures

- Fixes (base on comments) - import check - eslit command executed - npm run test command executed - no it.only in the commit

- Option name rollback - comment left - Option for customized the test error when hook fails (put off for next ticket)

- bad merge from main (imports)

- Separate hook and test status getter - review of test cases for fixtures. - fixtures cannot be failed the worst status could be broken. Signed-off-by: Manuel Lara <lararojas.mr@gmail.com>

The _startAppium is stucking whitout timeout when the promise emit 'data' but the data not includes the specific text. I've set a timeout y default in 60sec by security, and implemented a validation to reject only once at the first failure. Add some comments.

lararojasmr · 2024-01-26T02:23:51Z

@christian-bromann please check this changes.

christian-bromann

Some last comments/suggestions.

packages/wdio-appium-service/src/launcher.ts

Co-authored-by: Christian Bromann <git@bromann.dev>

Signed-off-by: Lara, Manuel <lararojas.mr@gmail.com>

…to bugfix/appium-start-function

christian-bromann

LGTM, thanks 👍

lararojasmr · 2024-01-29T17:55:29Z

LGTM, thanks 👍

@christian-bromann I saw some TC failing. Anything that I can do? - I've just update the branch.

Signed-off-by: Lara, Manuel <lararojas.mr@gmail.com>

…to bugfix/appium-start-function

Signed-off-by: Lara, Manuel <lararojas.mr@gmail.com>

lararojasmr · 2024-01-29T19:51:47Z

@christian-bromann I sent some fixes :) all test cases are passing now. Sorry hehe

christian-bromann

LGTM 👍 thanks!

wdio-bot · 2024-01-29T22:41:43Z

Hey lararojasmr 👋

Thank you for your contribution to WebdriverIO! Your pull request has been marked as an "Expensable" contribution.
We've sent you an email with further instructions on how to claim your expenses from our development fund. Please
make sure to check your spam folder as well. If you have any questions, feel free to reach out to us at expense@webdriver.io
or in the contributing channel on Discord.

We are looking forward to more contributions from you in the future 🙌

Have a nice day,
The WebdriverIO Team 🤖

picakia · 2024-01-31T12:50:17Z

Hey, I recently updated from 8.29.1 to 8.29.3 and it looks like this change introduced a bug with logging that generates countless "INFO @wdio/appium-service: Appium started with ID: 496446" messages as can be seen in this screenshot:

Reverting back to 8.29.1 resolves the issue

lararojasmr · 2024-01-31T15:16:25Z

Hey, let me check this now. Thanks for reporting it.On 31 Jan 2024, at 09:50, Filip ***@***.***> wrote: Hey, I recently updated from 8.29.1 to 8.29.3 and it looks like this change introduced a bug with logging that generates countless "INFO @wdio/appium-service: Appium started with ID: 496446" messages as can be seen in this screenshot: image.png (view on web) Reverting back to 8.29.1 resolves the issue —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>

lararojasmr · 2024-01-31T16:53:38Z

@picakia @christian-bromann A new ticket should be created for this issue, I have the solution but I not sure about how proceed here.

christian-bromann · 2024-01-31T17:07:16Z

@lararojasmr feel free to just raise a PR, no need to create an extra issue for it if you have a solution already. Thanks for looking into it.

lararojasmr · 2024-01-31T17:29:25Z

@christian-bromann sorry to late I created a new Issue haha. But I've added the fixes in this PR ☝🏼

…#12095

* Update to RN 0.73 * Change files * RN 73 requires node 18 * Add 72 profile * Update wdio/appium etc to get it running on node 18 * fix * fix * Update podfile locks * fix * fix * fix * see if building fixes resolution of focus-zone by e2e tests * Try specifying reports dir with full path * Try debug level reporting to identify error creating logs * try enabling more logs * alternate way to specify logfile. * downgrade @wdio/appium-service to move before webdriverio/webdriverio#12095 * Test no-packager * yarn init-test-app (With cleanup) * Try install Java18 * fix * fix * Android E2E fixes * fix? * fix * fix * Add more logging for E2E android test failure * more logs * more log * Use actual scroll region, to avoid OS swipe gestures * Increase android time out to navigate to page * fix * remove extra try/catch and logging * Fix windows build * test windows PR without node install --------- Co-authored-by: Saad Najmi <sanajmi@microsoft.com>

lararojasmr and others added 28 commits October 19, 2023 12:17

Allure Reporter Improvements. Adding support to fixtures for mocha/ja…

24e96e9

…smine. Update the Cucumber Step Mode. Change constants and option names.

Merge branch 'webdriverio:main' into main

19e2fc4

Fix test:depcheck in packages/wdio-allure-reporter/src/state.ts

32ea08c

Merge remote-tracking branch 'origin/main'

fefa9a1

Fixes:

415614f

- using 'getErrorFromFailedTest' to format error messages

Fixes:

ecb664d

- removing unnecessary code for mocha and jasmine

Fixes:

af78e05

- eslint and spaces before from.

Merge branch 'main' into main

0ba3011

Fixes:

62773cc

- Property 'labels' does not exist on type 'ExecutableItem'. - error TS2835: Relative import paths need explicit

Merge remote-tracking branch 'origin/main'

05f885b

Fixes:

7430e0c

- Allure Report Methods. - Eslint Fixes - Test Cases wdio-allure-reporter related

Fixes:

a3ea3bc

- Fix undefined in fixtures

Merge branch 'main' into main

f157a09

Update packages/wdio-allure-reporter/tests/allureFeatures.test.ts

3e6c5fe

Fixes:

d75439d

- Fixes (base on comments) - import check - eslit command executed - npm run test command executed - no it.only in the commit

Merge branch 'main' into main

377d1ee

Fixes:

6513708

- Option name rollback - comment left - Option for customized the test error when hook fails (put off for next ticket)

Merge remote-tracking branch 'origin/main'

d76702e

Merge branch 'main' into main

e8daf57

Merge branch 'webdriverio:main' into main

aca55e7

Fixes:

ddeed88

- bad merge from main (imports)

Fixes:

21d0f8d

- Separate hook and test status getter - review of test cases for fixtures. - fixtures cannot be failed the worst status could be broken. Signed-off-by: Manuel Lara <lararojas.mr@gmail.com>

Merge branch 'main' into main

59bec98

Merge branch 'main' into main

a4222a8

Merge branch 'webdriverio:main' into main

e9969f5

Merge branch 'main' into main

bd93798

Git webdriverio#12094

a99f04e

The _startAppium is stucking whitout timeout when the promise emit 'data' but the data not includes the specific text. I've set a timeout y default in 60sec by security, and implemented a validation to reject only once at the first failure. Add some comments.

Removing all allure changes. They were sending to other branch.

cf4e13a

lararojasmr changed the title ~~#12094 Bugfix/appium start function~~ webdriverio#12094 Bugfix/appium start function Jan 26, 2024

Merge branch 'main' into bugfix/appium-start-function

5972497

lararojasmr requested a review from christian-bromann January 29, 2024 07:32

christian-bromann reviewed Jan 29, 2024

View reviewed changes

packages/wdio-appium-service/src/launcher.ts Outdated Show resolved Hide resolved

packages/wdio-appium-service/src/launcher.ts Outdated Show resolved Hide resolved

packages/wdio-appium-service/src/launcher.ts Outdated Show resolved Hide resolved

lararojasmr and others added 4 commits January 29, 2024 13:58

Update packages/wdio-appium-service/src/launcher.ts

276a7ac

Co-authored-by: Christian Bromann <git@bromann.dev>

Update packages/wdio-appium-service/src/launcher.ts - Type

8e4cd39

Co-authored-by: Christian Bromann <git@bromann.dev>

Create a const for timeout and set the timeout in 30 sec by default.

a4babda

Signed-off-by: Lara, Manuel <lararojas.mr@gmail.com>

Merge remote-tracking branch 'origin/bugfix/appium-start-function' in…

33d4517

…to bugfix/appium-start-function

lararojasmr requested a review from christian-bromann January 29, 2024 17:04

Merge branch 'main' into bugfix/appium-start-function

5b24950

christian-bromann approved these changes Jan 29, 2024

View reviewed changes

Merge branch 'main' into bugfix/appium-start-function

a963d72

lararojasmr added 3 commits January 29, 2024 15:02

Fix eslint errors.

5e83d99

Signed-off-by: Lara, Manuel <lararojas.mr@gmail.com>

Merge remote-tracking branch 'origin/bugfix/appium-start-function' in…

d75ea91

…to bugfix/appium-start-function

Fix test cases, changes in the method to cover all missing cases.

060f15d

Signed-off-by: Lara, Manuel <lararojas.mr@gmail.com>

christian-bromann approved these changes Jan 29, 2024

View reviewed changes

christian-bromann added the PR: Bug Fix 🐛 PRs that contain bug fixes label Jan 29, 2024

christian-bromann merged commit 91f261e into webdriverio:main Jan 29, 2024

wdio-bot added the Expensable $50 💸 label Jan 29, 2024

lararojasmr mentioned this pull request Jan 31, 2024

[🐛 Bug]: [wdio-appium service]: Excessive Repeated Logging of Appium Connection Status after Update to 8.29.3 #12134

Closed

3 tasks

christian-bromann mentioned this pull request Feb 21, 2024

[🐛 Bug]: Appium service is not starting against local emulator using latest versions. #12094

Closed

3 tasks

acoates-ms added a commit to microsoft/fluentui-react-native that referenced this pull request Apr 4, 2024

downgrade @wdio/appium-service to move before webdriverio/webdriverio…

cc53fda

…#12095

Uh oh!

Conversation

lararojasmr commented Jan 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Types of changes

Checklist

Further comments

Reviewers: @webdriverio/project-committers

Uh oh!

lararojasmr commented Jan 26, 2024

Uh oh!

christian-bromann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

christian-bromann left a comment

Choose a reason for hiding this comment

Uh oh!

lararojasmr commented Jan 29, 2024

Uh oh!

lararojasmr commented Jan 29, 2024

Uh oh!

christian-bromann left a comment

Choose a reason for hiding this comment

Uh oh!

wdio-bot commented Jan 29, 2024

Uh oh!

picakia commented Jan 31, 2024

Uh oh!

lararojasmr commented Jan 31, 2024 via email

Uh oh!

lararojasmr commented Jan 31, 2024

Uh oh!

christian-bromann commented Jan 31, 2024

Uh oh!

lararojasmr commented Jan 31, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lararojasmr commented Jan 26, 2024 •

edited

Loading