Skip to content

feat: Swap check for Widget and WebElement - Allows Widget extensions to im… #2277

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 10 commits into from
Apr 28, 2025

Conversation

iFatRain
Copy link
Contributor

Change list

  • Swaps the order of decoration checking from WebElement first to Widget first.

Types of changes

Changes Widget init. to enable a little more flexibility in use cases.

  • No changes in production code.
  • 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)

Details

  • Currently, if an user wishes for their Widget extension to implement the WebElement interface and thus be used interchangeably as a WebElement (removing the need to always call getWrappedElement() when you wish to do so). However current order of operations will always check for WebElement first (Which works when WebElement is implemented) and thus prevents correct Widget decoration from happening. This change, which may come at a slight performance cost when normal WebElements are used, should enable Widget extensions to implement the WebElement interface.

Copy link

linux-foundation-easycla bot commented Mar 10, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@iFatRain iFatRain changed the title Swap check for Widget and WebElement - Allows Widget extensions to im… feat: Swap check for Widget and WebElement - Allows Widget extensions to im… Mar 10, 2025
@iFatRain
Copy link
Contributor Author

There are some slight changes to test but also the DefaultStubElement now implements the WebElement interface;

Prior to the changes contained within, this should've caused all Widgets to then register as only WebElements and NOT widgets; so if widget tests still work this is a good sign that they still function as Widgets. Ive also added some specific assertions to verify that the Widgets are instances of the WebElement interface, indicating that the widgets can be passed as WebElements without need a call to .getWrappedElement(). Finally, there is a change to the Browser test which just uses regular WebElements and the AppiumFieldDecorator to verify that simple elements will not be mistakenly set to be Widgets.

@iFatRain
Copy link
Contributor Author

Had to make some quick updates for a silly oversight as well as a thing about using * in imports

@iFatRain
Copy link
Contributor Author

@valfirst @mykola-mokhnach Is there some kind of documentation on a specific ordering for imports? Builds keep failing due to import order and I don't want to keep playing the build and wait for failure to keep making changes to that.

@valfirst
Copy link
Collaborator

@iFatRain here are rules:

<module name="CustomImportOrder">
<property name="customImportOrderRules" value="THIRD_PARTY_PACKAGE###SPECIAL_IMPORTS###STANDARD_JAVA_PACKAGE###STATIC"/>
<property name="specialImportsRegExp" value="^javax\."/>
<property name="standardPackageRegExp" value="^java\."/>
<property name="sortImportsInGroupAlphabetically" value="true"/>
<property name="separateLineBetweenGroups" value="false"/>
</module>

(and alphabetical order inside groups)

@iFatRain
Copy link
Contributor Author

Interesting that only the flutter-ios tests are now seeming to fail;

I didn't touch anything that im aware of which could impact those tests, since the changes were PageFactory related only?

@valfirst
Copy link
Collaborator

@iFatRain These are flaky tests, is your PR ready? can it be moved out o draft?

@iFatRain
Copy link
Contributor Author

iFatRain commented Apr 25, 2025

@valfirst I can move it out of draft if the changes to test cases are sufficient for what was asked.

I modified the DefaultStubWidget within the Widget tests to implement WebElement to show that it didn't adversely impact the creation and test cases for Widgets, while additional showcasing that they can be leveraged as element themselves, adding a check in one of the tests to show that the widget is an instance of WebElement;

@iFatRain iFatRain marked this pull request as ready for review April 25, 2025 21:45
@mykola-mokhnach mykola-mokhnach merged commit 3e5f9e2 into appium:master Apr 28, 2025
7 checks passed
@KazuCocoa KazuCocoa added the size:S contribution size: S label May 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:S contribution size: S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants