Skip to content

Feature/appium terminate#2

Merged
malto101 merged 3 commits intomozarkai:mainfrom
davidamo9:feature/appium_terminate
Apr 4, 2025
Merged

Feature/appium terminate#2
malto101 merged 3 commits intomozarkai:mainfrom
davidamo9:feature/appium_terminate

Conversation

@davidamo9
Copy link
Contributor

@davidamo9 davidamo9 commented Apr 3, 2025

  • added Appium session termination from session manager
  • merged locate_using_index to locate and deprecated it
  • pre-commit check

@malto101 malto101 requested a review from Copilot April 3, 2025 05:43
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new method for terminating Appium sessions while refactoring element location methods across OCR and image processing modules to support an optional index parameter, and deprecates the old locate_using_index methods.

  • Refactored element location functions to accept an optional index parameter.
  • Removed deprecated detection methods and updated error handling across multiple element source modules.
  • Renamed the Appium session termination method from end_session to terminate and updated its usage in the session manager.

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
optics_framework/engines/vision_models/ocr_models/pytesseract.py Updated locate & find_element methods; removed deprecated detect and locate_using_index methods.
optics_framework/engines/vision_models/ocr_models/googlevision.py Modified to use a unified locate API with index support; potential uninitialized variable usage.
optics_framework/engines/vision_models/ocr_models/easyocr.py Refactored to include index parameter in locate and find_element; potential undefined variable issue.
optics_framework/engines/vision_models/image_models/templatematch.py Updated locate and find_element_index to support an optional index parameter.
optics_framework/engines/elementsources/* Removed locate_using_index methods; adjusted locate and error handling accordingly.
optics_framework/engines/drivers/appium.py Renamed end_session to terminate to standardize session termination.
optics_framework/common/* Updated abstract definitions for locate and find_element to accept an optional index parameter.
optics_framework/common/session_manager.py Updated terminate_session to use the new terminate method on the driver.
optics_framework/api/action_keyword.py Adjusted calls to use the new locate API without the deprecated locate_using_index.
optics_framework/engines/drivers/appium_UI_helper.py Added a new get_locator_and_strategy_using_index helper for locating elements by index.
Comments suppressed due to low confidence (2)

optics_framework/engines/vision_models/ocr_models/googlevision.py:76

  • The variables 'top_left' and 'bottom_right' are not defined in this scope. Consider extracting these values from the OCR results (e.g., using (x, y) and (x+w, y+h)) to properly annotate the bounding box.
matches.append(((center_x, center_y), (top_left, bottom_right)))

optics_framework/engines/vision_models/ocr_models/easyocr.py:85

  • The variable 'detected_texts' is not defined in this function; it appears that 'matches' should be used instead. Update the condition to check for an empty matches list.
if not detected_texts:

"""Terminates a session and cleans up resources."""
self.sessions.pop(session_id, None)
session = self.sessions.pop(session_id, None)
session.driver.terminate() No newline at end of file
Copy link

Copilot AI Apr 3, 2025

Choose a reason for hiding this comment

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

There's a potential bug if the session with the given session_id does not exist, as 'session' could be null before calling driver.terminate(). Consider adding a null check before attempting to terminate the driver.

Suggested change
session.driver.terminate()
if session is not None:
session.driver.terminate()

Copilot uses AI. Check for mistakes.
@malto101
Copy link
Member

malto101 commented Apr 3, 2025

@davidamo9 could you run pre-commit and rebase it once more

@davidamo9 davidamo9 requested a review from malto101 April 4, 2025 09:36
@malto101 malto101 merged commit 30d3ace into mozarkai:main Apr 4, 2025
sheshnath1st pushed a commit to sheshnath1st/optics-framework that referenced this pull request Jan 12, 2026
# This is the 1st commit message:

test(playwright): add unit test for app launch

# This is the commit message mozarkai#2:

test(playwright): verify youtube search box is clickable

# This is the commit message mozarkai#3:

test(playwright): verify youtube search box click with delay

# This is the commit message mozarkai#4:

test(playwright): change config sample

# This is the commit message mozarkai#5:

test(playwright): add youtube search box click and sleep test

# This is the commit message mozarkai#6:

test(playwright): add comprehensive framework feature coverage launch_app , add_and_get_element , clear_element_text , _flow , validation_methods , get_text
sheshnath1st pushed a commit to sheshnath1st/optics-framework that referenced this pull request Jan 12, 2026
# This is the 1st commit message:

refactor: centralize retry logic to remove duplication

# This is the commit message mozarkai#2:

refactor: centralize locator resolve+exists to eliminate duplication

# This is the commit message mozarkai#3:

refactor: collapse retry logic to single loop to eliminate duplication

# This is the commit message mozarkai#4:

refactor: remove duplicated assertion retry logic

# This is the commit message mozarkai#5:

refactor: remove duplicated percentage using method about

# This is the commit message mozarkai#6:

refactor: remove duplicated percentage using method about

# This is the commit message mozarkai#7:

refactor: remove duplicated percentage using method about

# This is the commit message mozarkai#8:

refactor: remove duplicated percentage using method about
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants