Skip to content

chore(appium-service): filter out Debugger attached as errorMessage#13913

Merged
christian-bromann merged 5 commits intowebdriverio:mainfrom
Delta456:appium_debug
Dec 31, 2024
Merged

chore(appium-service): filter out Debugger attached as errorMessage#13913
christian-bromann merged 5 commits intowebdriverio:mainfrom
Delta456:appium_debug

Conversation

@Delta456
Copy link
Contributor

@Delta456 Delta456 commented Nov 24, 2024

Proposed changes

Types of changes

  • 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

Further comments

Reviewers: @webdriverio/project-committers

Fixes #12227 by filtering out Debugger attached in log error as said by @christian-bromann in #12227 (comment)

With the fix, ERROR: Debugger attached is now gone.

image

@Delta456
Copy link
Contributor Author

Delta456 commented Nov 24, 2024

I am not sure how will I test this change and if this needs to be backported to v8 as well

@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 24, 2024 •
edited
Loading

Open in Stackblitz

eslint-plugin-wdio

npm i https://pkg.pr.new/webdriverio/webdriverio/eslint-plugin-wdio@13913

@wdio/allure-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/allure-reporter@13913

@wdio/browser-runner

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/browser-runner@13913

@wdio/appium-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/appium-service@13913

@wdio/browserstack-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/browserstack-service@13913

@wdio/cli

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/cli@13913

@wdio/concise-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/concise-reporter@13913

@wdio/config

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/config@13913

@wdio/cucumber-framework

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/cucumber-framework@13913

@wdio/dot-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/dot-reporter@13913

@wdio/firefox-profile-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/firefox-profile-service@13913

@wdio/globals

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/globals@13913

@wdio/jasmine-framework

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/jasmine-framework@13913

@wdio/json-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/json-reporter@13913

@wdio/junit-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/junit-reporter@13913

@wdio/lighthouse-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/lighthouse-service@13913

@wdio/local-runner

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/local-runner@13913

@wdio/logger

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/logger@13913

@wdio/mocha-framework

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/mocha-framework@13913

@wdio/protocols

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/protocols@13913

@wdio/repl

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/repl@13913

@wdio/reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/reporter@13913

@wdio/runner

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/runner@13913

@wdio/sauce-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/sauce-service@13913

@wdio/shared-store-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/shared-store-service@13913

@wdio/smoke-test-cjs-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/smoke-test-cjs-service@13913

@wdio/smoke-test-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/smoke-test-reporter@13913

@wdio/smoke-test-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/smoke-test-service@13913

@wdio/spec-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/spec-reporter@13913

@wdio/static-server-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/static-server-service@13913

@wdio/sumologic-reporter

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/sumologic-reporter@13913

@wdio/testingbot-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/testingbot-service@13913

@wdio/types

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/types@13913

@wdio/utils

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/utils@13913

@wdio/webdriver-mock-service

npm i https://pkg.pr.new/webdriverio/webdriverio/@wdio/webdriver-mock-service@13913

webdriver

npm i https://pkg.pr.new/webdriverio/webdriverio/webdriver@13913

webdriverio

npm i https://pkg.pr.new/webdriverio/webdriverio@13913

commit: ec37e29

christian-bromann
christian-bromann requested changes Nov 25, 2024
View reviewed changes
Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

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

Can we add a unit test for this?

@Delta456
Copy link
Contributor Author

Delta456 commented Nov 25, 2024

I am not sure how we will test these changes Do you know how 🤔

@christian-bromann
Copy link
Member

christian-bromann commented Nov 25, 2024

I recommend take a look at these unit tests and add one that mocks the child process in a similar way but triggers the stderr event to be called so you can test your callback.

@christian-bromann
Copy link
Member

christian-bromann commented Dec 10, 2024

@Delta456 if you have trouble with the unit test, feel free to schedule a 1:1 session through our Open Office Hours, thanks.

@Delta456
Copy link
Contributor Author

Delta456 commented Dec 20, 2024

@Delta456 if you have trouble with the unit test, feel free to schedule a 1:1 session through our Open Office Hours, thanks.

Hi @christian-bromann, I have scheduled a session with you. Hope to see you there

@Delta456
Copy link
Contributor Author

Delta456 commented Dec 27, 2024

The test is now added. Thanks to the appium team!

christian-bromann
christian-bromann requested changes Dec 27, 2024
View reviewed changes
Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

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

One suggestion on the unit test.

christian-bromann
christian-bromann reviewed Dec 30, 2024
View reviewed changes
Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

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

Thanks, the code looks good now.

Can you please help me understand how filtering out the error message helps resolve the issue? I think it makes sense adding a comment to the if statement to inform developers why we are not logging in this case.

christian-bromann
christian-bromann approved these changes Dec 31, 2024
View reviewed changes
Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Thanks so much!

@christian-bromann christian-bromann added the PR: Bug Fix 🐛 PRs that contain bug fixes label Dec 31, 2024
@christian-bromann christian-bromann merged commit b60ab80 into webdriverio:main Dec 31, 2024
@Delta456 Delta456 deleted the appium_debug branch January 2, 2025 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@christian-bromann christian-bromann christian-bromann approved these changes

+1 more reviewer

@SrinivasanTarget SrinivasanTarget SrinivasanTarget left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

PR: Bug Fix 🐛 PRs that contain bug fixes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[🐛 Bug]: [appium-service] Running Appium WDIO spec from VS Code debugger is rarely working (extremely flaky)

3 participants

@Delta456 @christian-bromann @SrinivasanTarget