Skip to content

[java] Process Selenium Manager output as JSON #11663

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Mar 15, 2023
Merged

[java] Process Selenium Manager output as JSON #11663

merged 3 commits into from
Mar 15, 2023

Conversation

bonigarcia
Copy link
Member

@bonigarcia bonigarcia commented Feb 16, 2023 •
edited
Loading

Description

This PR makes the Java bindings calls Selenium Manager using the flag --output json (implemented in #11531). Then, the output is parsed using with GSON. From the parsed output, the logs messages from Selenium Manager with WARN level are displayed to the user (using LOG.warning in Java).

Motivation and Context

This PR is the Java implementation for #11365.

NOTE: To test this feature, the Selenium Manager binary (located in common\manager) should be updated.

Types of changes

  • Bug fix (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 change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
@bonigarcia bonigarcia force-pushed the se_mgr_java_json_output branch 2 times, most recently from cad08ad to aa4ac9d Compare February 16, 2023 10:08
@codecov-commenter
Copy link

codecov-commenter commented Feb 16, 2023 •
edited
Loading

Codecov Report

Patch and project coverage have no change.

Comparison is base (e6a7987) 54.89% compared to head (f03ef58) 54.89%.

❗ Current head f03ef58 differs from pull request most recent head 1ee4133. Consider uploading reports for the commit 1ee4133 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files 
@@ Coverage Diff @@ ## trunk #11663 +/- ## ======================================= Coverage 54.89% 54.89% ======================================= Files 85 85 Lines 5684 5684 Branches 231 231 ======================================= Hits 3120 3120 Misses 2333 2333 Partials 231 231

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.
diemol
diemol requested changes Feb 17, 2023
View reviewed changes
Copy link
Member

@diemol diemol left a comment

Choose a reason for hiding this comment

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

I made changes so Java uses Selenium Manager to run tests, do you think we can add a test for this? It can be just with the INFO.
@diemol diemol force-pushed the se_mgr_java_json_output branch from aa4ac9d to 0b59c47 Compare February 17, 2023 11:26
@bonigarcia bonigarcia force-pushed the se_mgr_java_json_output branch from 0b59c47 to db9e0da Compare February 20, 2023 10:04
@bonigarcia
Copy link
Member Author

I made changes so Java uses Selenium Manager to run tests, do you think we can add a test for this? It can be just with the INFO.

Any Java test using Selenium Manager is going to use this JSON parsing.

What do you mean by "just with the INFO"?
@bonigarcia bonigarcia added the C-java Java Bindings label Feb 20, 2023
@bonigarcia bonigarcia force-pushed the se_mgr_java_json_output branch from db9e0da to 1ee4133 Compare March 14, 2023 17:20
@bonigarcia
Copy link
Member Author

@diemol This PR had conflicts, but I have just resolved it. Can it be merged?
diemol
diemol approved these changes Mar 15, 2023
View reviewed changes
Copy link
Member

@diemol diemol left a comment

Choose a reason for hiding this comment

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

Yes, thank you!
@bonigarcia bonigarcia merged commit 1de6a52 into trunk Mar 15, 2023
@bonigarcia bonigarcia deleted the se_mgr_java_json_output branch March 15, 2023 10:42
alpatron pushed a commit to alpatron/selenium that referenced this pull request Mar 15, 2023
@bonigarcia @diemol @alpatron
[java] Process Selenium Manager output as JSON (SeleniumHQ#11663)
b4d4ed7 
* [java] Process Selenium Manager output as JSON * [java] Use internal JSON parser instead of GSON --------- Co-authored-by: Diego Molina <diemol@users.noreply.github.com>
titusfortner
titusfortner reviewed Mar 15, 2023
View reviewed changes
java/src/org/openqa/selenium/manager/SeleniumManager.java
SeleniumManagerJsonOutput jsonOutput = new Json().toType(output,
SeleniumManagerJsonOutput.class);
jsonOutput.logs.stream().filter(log -> log.level.equalsIgnoreCase(WARN))
.forEach(log -> LOG.warning(log.message));
Copy link
Member

Choose a reason for hiding this comment

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

@bonigarcia / @diemol can we log the debug information to LOG.FINE here? Is this the right place?
Copy link
Member

Choose a reason for hiding this comment

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

Yes, we can do that.
@diemol
Copy link
Member

diemol commented Mar 16, 2023

Seems we missed to update the Selenium Manager binary before merging this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-java Java Bindings

4 participants

@bonigarcia @codecov-commenter @diemol @titusfortner