In @wdio/appium-service, kill entire Appium process tree by samuelfreiberg · Pull Request #13151 · webdriverio/webdriverio

samuelfreiberg · 2024-07-11T23:24:49Z

Proposed changes

This change kills the entire Appium process tree in the onComplete() callback. The current method is only doing:
this._process.kill(). This doesn't take into account the spawned process' from Appium.

Currently, there is a lingering Node process after executing testing. I found that this node process is spawned from WebDriverIO, specifically for Appium: "C:\Program Files\nodejs\node.exe" C:\my_local_project_path\node_modules\appium\index.js --base-path /

This causes the next test execution in the same cmd prompt to fail with:
2024-07-01T22:39:19.725Z ERROR @wdio/appium-service: Appium exited before timeout (exit code: 1) [HTTP] Could not start REST http interface listener. The requested port may already be in use. Please make sure there is no other instance of this server running already.

Types of changes

My solution is to take a dependency on the tree-kill NPM package. This is a simple package that allows us to kill all process' in a tree. I'm using this._process.pid to kill the entire process tree.

I've confirmed there is no more lingering Node process after test execution

However, as I stated in the issue, we're still getting ERROR @wdio/appium-service: Appium exited before timeout (exit code: null). I can't figure out why. I don't see any actual user bad from this - the tests still pass successfully. Nothing in the Appium logs indicate anything bad, either - and now there's no lingering node process.

Since there's no user bad, and the process is getting shut down successfully, I'm going to change this from a log.error to a log.warn.

Polish (an improvement for an existing feature)
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 (improvements to the project's docs)
Specification changes (updates to WebDriver command specifications)
Internal updates (everything related to internal scripts, governance documentation and CI files)

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 the necessary documentation (if appropriate)
I have added proper type definitions for new commands (if appropriate)

Backport Request

//: # (The current main branch is the development branch for WebdriverIO v9. If your change should be released to the current major version of WebdriverIO (v8), please raise another PR with the same changes against the v8 branch.)

This change is solely for v9 and doesn't need to be back-ported
Back-ported PR at #XXXXX
I will backport this once it gets approved

Further comments

Reviewers: @webdriverio/project-committers

christian-bromann · 2024-07-12T14:52:43Z

@samuelfreiberg thanks for raising the PR ❤️ do you mind resolving the conflict in the pnpm-lock.yaml?

samuelfreiberg · 2024-07-12T20:42:35Z

@samuelfreiberg thanks for raising the PR ❤️ do you mind resolving the conflict in the pnpm-lock.yaml?

No problem! And just did fix the conflict

christian-bromann · 2024-07-15T15:19:28Z

@samuelfreiberg thanks for the changes. I will take a look today at the build issues we face and will update your branch once these are resolved to see if this patch is good to go.

samuelfreiberg · 2024-07-16T16:26:39Z

@samuelfreiberg thanks for the changes. I will take a look today at the build issues we face and will update your branch once these are resolved to see if this patch is good to go.

Sounds good - thanks

christian-bromann · 2024-07-19T06:48:32Z

@samuelfreiberg mind rebasing your branch, I pushed some fixes to the main branch.

christian-bromann · 2024-07-31T21:10:06Z

@samuelfreiberg mind taking a look a the following test:

 FAIL  packages/wdio-appium-service/tests/launcher.test.ts > Appium launcher > onComplete > should call process.kill
AssertionError: expected "spy" to be called at least once

 ❯ packages/wdio-appium-service/tests/launcher.test.ts:635:30
Error: AssertionError: expected "spy" to be called at least once
 ❯ packages/wdio-appium-service/tests/launcher.test.ts:635:30


    633|             launcher['_process']!.kill = vi.fn()
    634|             launcher.onComplete()
    635|             expect(treeKill).toHaveBeenCalled()
       |                              ^
    636|         })
    637|

samuelfreiberg · 2024-08-01T21:13:44Z

@samuelfreiberg mind taking a look a the following test:

 FAIL  packages/wdio-appium-service/tests/launcher.test.ts > Appium launcher > onComplete > should call process.kill
AssertionError: expected "spy" to be called at least once

 ❯ packages/wdio-appium-service/tests/launcher.test.ts:635:30
Error: AssertionError: expected "spy" to be called at least once
 ❯ packages/wdio-appium-service/tests/launcher.test.ts:635:30


    633|             launcher['_process']!.kill = vi.fn()
    634|             launcher.onComplete()
    635|             expect(treeKill).toHaveBeenCalled()
       |                              ^
    636|         })
    637|

Just updated

christian-bromann · 2024-08-02T15:33:15Z

mhm .. I still see 2 failing tests:

Error: AssertionError: expected "spy" to be called with arguments: []

Received: 

Number of calls: 0

 ❯ packages/wdio-appium-service/tests/launcher.test.ts:638:30

 FAIL  packages/wdio-appium-service/tests/launcher.test.ts > Appium launcher > onComplete > should call treeKill
AssertionError: expected "spy" to be called with arguments: []

Received: 

Number of calls: 0

 ❯ packages/wdio-appium-service/tests/launcher.test.ts:638:30
    636| 
    637|             // Verify treeKill is called with correct parameters
    638|             expect(treeKill).toHaveBeenCalledWith()
       |                              ^
    639|         })
    640|

samuelfreiberg · 2024-08-02T17:29:48Z

mhm .. I still see 2 failing tests:

Error: AssertionError: expected "spy" to be called with arguments: []

Received: 

Number of calls: 0

 ❯ packages/wdio-appium-service/tests/launcher.test.ts:638:30

 FAIL  packages/wdio-appium-service/tests/launcher.test.ts > Appium launcher > onComplete > should call treeKill
AssertionError: expected "spy" to be called with arguments: []

Received: 

Number of calls: 0

 ❯ packages/wdio-appium-service/tests/launcher.test.ts:638:30
    636| 
    637|             // Verify treeKill is called with correct parameters
    638|             expect(treeKill).toHaveBeenCalledWith()
       |                              ^
    639|         })
    640|

Looks like it's passing now. I had to mock up the entire _process in order to set the pid

christian-bromann

LGTM 👍

christian-bromann · 2024-08-04T05:49:01Z

The one failure is an known issue we are trying to fix, will move ahead and merge.

@samuelfreiberg thanks so much, mind raising the same PR against the v8 branch?

wdio-bot · 2024-08-04T05:49:47Z

Hey samuelfreiberg 👋

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 🤖

Fixing syntax

521920f

Merge branch 'main' into main

eba3c72

samuelfreiberg added 2 commits July 12, 2024 14:25

Updating appium launcher test to check for treeKill() call

f4008b9

Merge branch 'main' of https://github.com/samuelfreiberg/webdriverio

64741b5

Merging with main

f256e76

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

Alter test to check for treeKill

27f55c2

samuelfreiberg added 2 commits August 2, 2024 10:07

Import fixes

94a8279

Checking for correct params

748ebf4

christian-bromann mentioned this pull request Aug 3, 2024

[🐛 Bug]: ERROR @wdio/appium-service: Appium exited before timeout (exit code: null) #13216

Closed

3 tasks

christian-bromann approved these changes Aug 4, 2024

View reviewed changes

christian-bromann merged commit fe2eca6 into webdriverio:main Aug 4, 2024

wdio-bot added the Expensable $50 💸 label Aug 4, 2024

This was referenced Aug 12, 2024

[V8 PORT] In @wdio/appium-service, kill entire Appium process tree #13365

Merged

Fix Appium Shutdown Error #13366

Merged

[V8 Backport] Fix Appium Shutdown Error #13379

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

In @wdio/appium-service, kill entire Appium process tree#13151

In @wdio/appium-service, kill entire Appium process tree#13151
christian-bromann merged 8 commits intowebdriverio:mainfrom
samuelfreiberg:main

samuelfreiberg commented Jul 11, 2024

Uh oh!

christian-bromann commented Jul 12, 2024

Uh oh!

samuelfreiberg commented Jul 12, 2024

Uh oh!

christian-bromann commented Jul 15, 2024

Uh oh!

samuelfreiberg commented Jul 16, 2024

Uh oh!

christian-bromann commented Jul 19, 2024

Uh oh!

christian-bromann commented Jul 31, 2024

Uh oh!

samuelfreiberg commented Aug 1, 2024

Uh oh!

christian-bromann commented Aug 2, 2024

Uh oh!

samuelfreiberg commented Aug 2, 2024

Uh oh!

christian-bromann left a comment

Uh oh!

christian-bromann commented Aug 4, 2024

Uh oh!

wdio-bot commented Aug 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

samuelfreiberg commented Jul 11, 2024

Proposed changes

Types of changes

Checklist

Backport Request

Further comments

Reviewers: @webdriverio/project-committers

Uh oh!

christian-bromann commented Jul 12, 2024

Uh oh!

samuelfreiberg commented Jul 12, 2024

Uh oh!

christian-bromann commented Jul 15, 2024

Uh oh!

samuelfreiberg commented Jul 16, 2024

Uh oh!

christian-bromann commented Jul 19, 2024

Uh oh!

christian-bromann commented Jul 31, 2024

Uh oh!

samuelfreiberg commented Aug 1, 2024

Uh oh!

christian-bromann commented Aug 2, 2024

Uh oh!

samuelfreiberg commented Aug 2, 2024

Uh oh!

christian-bromann left a comment

Choose a reason for hiding this comment

Uh oh!

christian-bromann commented Aug 4, 2024

Uh oh!

wdio-bot commented Aug 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants