-
Notifications
You must be signed in to change notification settings - Fork 189
feat: Add WebSocket-based log broadcast support for Android #969
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
Merged
Merged
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
525defb
feat: Implement logcat message broadcasting via WebSocket in AndroidD…
Dor-bl f174355
test: Add integration tests for logcat message broadcasting in Androi…
Dor-bl 1ba8dc6
test: Enhance logcat broadcast error handling in LogcatBroadcastTests
Dor-bl 29f6149
test: Add LogcatBroadcastTests for logcat message broadcasting functi…
Dor-bl 18dbd45
chore: Remove unused using directives in IListensToLogcatMessages int…
Dor-bl 9a47096
refactor: Use 'using' statement for errorSemaphore in CanHandleErrors…
Dor-bl aa1dc9e
test: Improve error handling in StopLogcatBroadcast method
Dor-bl 9b4e34e
feat: Implement IDisposable interface in StringWebSocketClient for re…
Dor-bl 8fb8e98
feat: Enhance IDisposable implementation in StringWebSocketClient for…
Dor-bl 297e8d7
refactor: Replace static LogcatClient with instance variable for impr…
Dor-bl 2f92495
fix: Convert handler collections to arrays before invocation to preve…
Dor-bl f5ea24e
fix: Handle WebSocketException and TaskCanceledException in connectio…
Dor-bl 96e42b2
feat: Update StartLogcatBroadcast methods to async for improved perfo…
Dor-bl 6b6f00b
feat: Update CanAddAndRemoveMultipleListeners test to use async for i…
Dor-bl aba9987
fix: Remove unnecessary blank lines in CanAddAndRemoveMultipleListene…
Dor-bl 1313907
feat: Update VerifyLogcatListenerCanBeAssigned test to async for impr…
Dor-bl b373cbb
refactor: Simplify CanAddAndRemoveMultipleListeners test by using 'us…
Dor-bl edcd995
feat: Update LogcatBroadcast tests to use async for improved performa…
Dor-bl 147506a
feat: Ensure receive task completion before closing WebSocket connection
Dor-bl bcfe603
feat: Enhance WebSocket connection handling with semaphore for thread…
Dor-bl 9817502
fix: Remove unnecessary whitespace in CanStartLogcatBroadcastWithCust…
Dor-bl 09701b3
refactor: Replace direct handler additions with dedicated methods in …
Dor-bl 4b5fe71
fix: Remove initialization of ClientWebSocket in StringWebSocketClien…
Dor-bl 4c98fbb
feat: Update StopLogcatBroadcast method to be asynchronous
Dor-bl 1a03163
fix: Use ToArray() on ErrorHandlers to avoid modification during iter…
Dor-bl 8b4f000
feat: Make StopLogcatBroadcast method asynchronous in tests
Dor-bl a1458c9
chore: Add remarks to StartLogcatBroadcast methods regarding temporar…
Dor-bl d0269c4
fix: Update StartLogcatBroadcast method to use loopback address
Dor-bl 38a9314
fix: Enhance WebDriverException message with timestamp for WebSocket …
Dor-bl 2694cbf
fix: Update host variable in CanStartLogcatBroadcastWithCustomHost te…
Dor-bl File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
92 changes: 92 additions & 0 deletions
92
src/Appium.Net/Appium/Android/Interfaces/IListensToLogcatMessages.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| //Licensed under the Apache License, Version 2.0 (the "License"); | ||
| //you may not use this file except in compliance with the License. | ||
| //See the NOTICE file distributed with this work for additional | ||
| //information regarding copyright ownership. | ||
| //You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| //Unless required by applicable law or agreed to in writing, software | ||
| //distributed under the License is distributed on an "AS IS" BASIS, | ||
| //WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| //See the License for the specific language governing permissions and | ||
| //limitations under the License. | ||
|
|
||
| using System; | ||
| using System.Threading.Tasks; | ||
|
|
||
| namespace OpenQA.Selenium.Appium.Android.Interfaces | ||
| { | ||
| /// <summary> | ||
| /// Interface for handling Android logcat message broadcasts via WebSocket. | ||
| /// </summary> | ||
| public interface IListensToLogcatMessages | ||
| { | ||
| /// <summary> | ||
| /// Start logcat messages broadcast via web socket. | ||
| /// This method assumes that Appium server is running on localhost and | ||
| /// is assigned to the default port (4723). | ||
| /// </summary> | ||
| Task StartLogcatBroadcast(); | ||
|
|
||
| /// <summary> | ||
| /// Start logcat messages broadcast via web socket. | ||
| /// This method assumes that Appium server is assigned to the default port (4723). | ||
| /// </summary> | ||
| /// <param name="host">The name of the host where Appium server is running.</param> | ||
| Task StartLogcatBroadcast(string host); | ||
|
|
||
| /// <summary> | ||
| /// Start logcat messages broadcast via web socket. | ||
| /// </summary> | ||
| /// <param name="host">The name of the host where Appium server is running.</param> | ||
| /// <param name="port">The port of the host where Appium server is running.</param> | ||
| Task StartLogcatBroadcast(string host, int port); | ||
|
|
||
| /// <summary> | ||
| /// Adds a new log messages broadcasting handler. | ||
| /// Several handlers might be assigned to a single server. | ||
| /// Multiple calls to this method will cause such handler | ||
| /// to be called multiple times. | ||
| /// </summary> | ||
| /// <param name="handler">A function, which accepts a single argument, which is the actual log message.</param> | ||
| void AddLogcatMessagesListener(Action<string> handler); | ||
|
|
||
| /// <summary> | ||
| /// Adds a new log broadcasting errors handler. | ||
| /// Several handlers might be assigned to a single server. | ||
| /// Multiple calls to this method will cause such handler | ||
| /// to be called multiple times. | ||
| /// </summary> | ||
| /// <param name="handler">A function, which accepts a single argument, which is the actual exception instance.</param> | ||
| void AddLogcatErrorsListener(Action<Exception> handler); | ||
|
|
||
| /// <summary> | ||
| /// Adds a new log broadcasting connection handler. | ||
| /// Several handlers might be assigned to a single server. | ||
| /// Multiple calls to this method will cause such handler | ||
| /// to be called multiple times. | ||
| /// </summary> | ||
| /// <param name="handler">A function, which is executed as soon as the client is successfully connected to the web socket.</param> | ||
| void AddLogcatConnectionListener(Action handler); | ||
|
|
||
| /// <summary> | ||
| /// Adds a new log broadcasting disconnection handler. | ||
| /// Several handlers might be assigned to a single server. | ||
| /// Multiple calls to this method will cause such handler | ||
| /// to be called multiple times. | ||
| /// </summary> | ||
| /// <param name="handler">A function, which is executed as soon as the client is successfully disconnected from the web socket.</param> | ||
| void AddLogcatDisconnectionListener(Action handler); | ||
|
|
||
| /// <summary> | ||
| /// Removes all existing logcat handlers. | ||
| /// </summary> | ||
| void RemoveAllLogcatListeners(); | ||
|
|
||
| /// <summary> | ||
| /// Stops logcat messages broadcast via web socket. | ||
| /// </summary> | ||
| Task StopLogcatBroadcast(); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| //Licensed under the Apache License, Version 2.0 (the "License"); | ||
| //you may not use this file except in compliance with the License. | ||
| //See the NOTICE file distributed with this work for additional | ||
| //information regarding copyright ownership. | ||
| //You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| //Unless required by applicable law or agreed to in writing, software | ||
| //distributed under the License is distributed on an "AS IS" BASIS, | ||
| //WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| //See the License for the specific language governing permissions and | ||
| //limitations under the License. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
|
|
||
| namespace OpenQA.Selenium.Appium.WebSocket | ||
| { | ||
| /// <summary> | ||
| /// Interface for handling WebSocket connections. | ||
| /// </summary> | ||
| public interface ICanHandleConnects | ||
| { | ||
| /// <summary> | ||
| /// Gets the list of web socket connection handlers. | ||
| /// </summary> | ||
| List<Action> ConnectionHandlers { get; } | ||
|
|
||
| /// <summary> | ||
| /// Register a new connection handler. | ||
| /// </summary> | ||
| /// <param name="handler">A callback function, which is going to be executed when web socket connection event arrives.</param> | ||
| void AddConnectionHandler(Action handler); | ||
|
|
||
| /// <summary> | ||
| /// Removes existing web socket connection handlers. | ||
| /// </summary> | ||
| void RemoveConnectionHandlers(); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| //Licensed under the Apache License, Version 2.0 (the "License"); | ||
| //you may not use this file except in compliance with the License. | ||
| //See the NOTICE file distributed with this work for additional | ||
| //information regarding copyright ownership. | ||
| //You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| //Unless required by applicable law or agreed to in writing, software | ||
| //distributed under the License is distributed on an "AS IS" BASIS, | ||
| //WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| //See the License for the specific language governing permissions and | ||
| //limitations under the License. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
|
|
||
| namespace OpenQA.Selenium.Appium.WebSocket | ||
| { | ||
| /// <summary> | ||
| /// Interface for handling WebSocket disconnections. | ||
| /// </summary> | ||
| public interface ICanHandleDisconnects | ||
| { | ||
| /// <summary> | ||
| /// Gets the list of web socket disconnection handlers. | ||
| /// </summary> | ||
| List<Action> DisconnectionHandlers { get; } | ||
|
|
||
| /// <summary> | ||
| /// Register a new web socket disconnect handler. | ||
| /// </summary> | ||
| /// <param name="handler">A callback function, which is going to be executed when web socket disconnect event arrives.</param> | ||
| void AddDisconnectionHandler(Action handler); | ||
|
|
||
| /// <summary> | ||
| /// Removes existing disconnection handlers. | ||
| /// </summary> | ||
| void RemoveDisconnectionHandlers(); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| //Licensed under the Apache License, Version 2.0 (the "License"); | ||
| //you may not use this file except in compliance with the License. | ||
| //See the NOTICE file distributed with this work for additional | ||
| //information regarding copyright ownership. | ||
| //You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| //Unless required by applicable law or agreed to in writing, software | ||
| //distributed under the License is distributed on an "AS IS" BASIS, | ||
| //WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| //See the License for the specific language governing permissions and | ||
| //limitations under the License. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
|
|
||
| namespace OpenQA.Selenium.Appium.WebSocket | ||
| { | ||
| /// <summary> | ||
| /// Interface for handling WebSocket errors. | ||
| /// </summary> | ||
| public interface ICanHandleErrors | ||
| { | ||
| /// <summary> | ||
| /// Gets the list of web socket error handlers. | ||
| /// </summary> | ||
| List<Action<Exception>> ErrorHandlers { get; } | ||
|
|
||
| /// <summary> | ||
| /// Register a new error handler. | ||
| /// </summary> | ||
| /// <param name="handler">A callback function, which accepts the received exception instance as a parameter.</param> | ||
| void AddErrorHandler(Action<Exception> handler); | ||
|
|
||
| /// <summary> | ||
| /// Removes existing error handlers. | ||
| /// </summary> | ||
| void RemoveErrorHandlers(); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| //Licensed under the Apache License, Version 2.0 (the "License"); | ||
| //you may not use this file except in compliance with the License. | ||
| //See the NOTICE file distributed with this work for additional | ||
| //information regarding copyright ownership. | ||
| //You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| //Unless required by applicable law or agreed to in writing, software | ||
| //distributed under the License is distributed on an "AS IS" BASIS, | ||
| //WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| //See the License for the specific language governing permissions and | ||
| //limitations under the License. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
|
|
||
| namespace OpenQA.Selenium.Appium.WebSocket | ||
| { | ||
| /// <summary> | ||
| /// Interface for handling WebSocket messages. | ||
| /// </summary> | ||
| /// <typeparam name="T">The type of message to handle.</typeparam> | ||
| public interface ICanHandleMessages<T> | ||
| { | ||
| /// <summary> | ||
| /// Gets the list of web socket message handlers. | ||
| /// </summary> | ||
| List<Action<T>> MessageHandlers { get; } | ||
|
|
||
| /// <summary> | ||
| /// Register a new message handler. | ||
| /// </summary> | ||
| /// <param name="handler">A callback function, which accepts the received message as a parameter.</param> | ||
| void AddMessageHandler(Action<T> handler); | ||
|
|
||
| /// <summary> | ||
| /// Removes existing message handlers. | ||
| /// </summary> | ||
| void RemoveMessageHandlers(); | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried BiDi instead of this extension command?
https://github.com/appium/appium-uiautomator2-driver?tab=readme-ov-file#mobile-startlogsbroadcast
https://github.com/search?q=org%3Aappium%20entryAdded&type=code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KazuCocoa, I attempted several different methods using BiDI, but unfortunately, none of them worked for me.
I'm also experiencing a problem with the BiDi test that you added recently (this might be connected to my issue).
When I attempt to establish a connection, I encounter this error:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you share the appium server-side log as well?
When i ran
dotnet test test/integration/Appium.Net.Integration.Tests.csproj -f net8.0 --filter "FullyQualifiedName~Appium.Net.Integration.Tests.Android.BiDiTests"on my local:appium was 3.1.1, uia2 driver was 6.1.0
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KazuCocoa, does BiDi require a specific minimum version of Appium/UIAutomator2 to function?
EDIT: Looks like I'm a bit behind with the versions:
https://gist.github.com/Dor-bl/95d17ca78974d6883da682fba6e49d10
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minimal could be appium 2 and uia2 3.7.10. Old ones might have issues though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take a look at the
log.entryAddedhanlding in each driver implementation tonightcc @mykola-mokhnach
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've opened an issue on the Appium server:
appium/appium#21741
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in appium/appium-android-driver#1031, the
contextproperty in thesourceobject is optional and should not be relied on. While I agree with @KazuCocoa in that there's no harm in adding it on the driver side, if Selenium is expecting this property, then this issue should be primarily fixed on the Selenium side.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nvborisenko, can we apply a fix on the Selenium side, following the above comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I can accept it. But why:
We subscribe like:
According spec
contextsproperty is not required: https://w3c.github.io/webdriver-bidi/#cddl-type-sessionsubscribeparameters. But appium server-side implementation requires it.