[KOB-52290] Fix Python script generation and add direct integration c…#59
[KOB-52290] Fix Python script generation and add direct integration c…#59mimosa767 wants to merge 1 commit intokobiton:masterfrom
Conversation
…overage Generated Python scripts were malformed in several ways: @classmethod landed outside the class body, indent levels drifted with each device block causing pytest to miss all but the first test function, finally was nested inside except causing an immediate SyntaxError, JS true/false appeared as Python literals, XPath selectors with double-quoted attributes broke string quoting, and multi-device if/elif/else branches were nested inside each other. Runtime template gaps were also fixed: missing AppiumBy/By imports in test_app.py, deprecated webdriver.Remote API updated to AppiumOptions in test_base.py, missing action helpers added (send_keys_to_active_element, swipe_on_element, press_button_multiple, rotate_screen, set_location), port handling corrected in config.py, and proxy auth switched from URL-embedded credentials to an Authorization header in proxy_server.py. A direct Python regression test (no gRPC) now generates a real zip from a 2-device fixture and runs ast.parse plus six structural checks over every .py file to protect against recurrence. Note: the pre-existing open-handles warning from the gRPC integration tests in generate-script.test.js is unrelated and left for a separate cleanup. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes Python (pytest) script generation issues in the Appium script generator/templates and adds a direct regression test to prevent malformed Python output from recurring.
Changes:
- Correct Python code generation indentation/structure (classmethod placement, try/except/finally alignment, module-level test functions, multi-device selector branching) and Python literal/string quoting.
- Update Python runtime templates (imports, Appium
Remoteoptions usage, new action helpers, proxy auth via header, config URL/port handling). - Add an integration test that generates a real zip from a 2-device fixture and validates all
.pyfiles viaast.parseplus structural checks.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/templates/python/test_base.py | Updates driver initialization to use AppiumOptions and adds new action helpers; adjusts element-finding to support multiple locators. |
| src/templates/python/test_app.py | Adds missing AppiumBy / By imports required by generated locator code. |
| src/templates/python/proxy_server.py | Switches proxy auth from URL-embedded creds to an Authorization header. |
| src/templates/python/config.py | Fixes URL auth generation when port is absent; provides basic-auth helper for proxy. |
| src/services/python.js | Fixes Python code generation indentation and literals; improves selector branching and quoting. |
| tests/resource/python-pytest-input.json | Adds a 2-device fixture input for Python pytest generation. |
| tests/integration/python-pytest.test.js | Adds a Node integration test that generates a zip and runs the Python validator. |
| tests/helpers/validate-python-output.py | Adds a Python validator to parse and structurally verify generated output. |
Comments suppressed due to low confidence (1)
src/services/python.js:113
- String desired-capability values are injected into Python using single quotes without escaping (e.g.,
'${key}': '${parsedValue}'). If a capability value contains a single quote, backslash, or newline, the generatedconfig.pycan become invalid Python or change meaning. Use the same string-escaping strategy as_getString()(orJSON.stringify-style escaping adapted for Python) when emitting Python string literals here.
if (typeof parsedValue === 'boolean') {
statement = `'${key}': ${parsedValue ? 'True' : 'False'}${suffix}`
}
else if (typeof parsedValue === 'string') {
statement = `'${key}': '${parsedValue}'${suffix}`
}
else {
statement = `'${key}': ${parsedValue}${suffix}`
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def find_visible_element(self, timeout_ms, locators): | ||
| wait = WebDriverWait(self._driver, timeout_ms / 1000) | ||
| return wait.until(EC.visibility_of_element_located((by, value))) | ||
| last_exception = None | ||
| for locator in locators: | ||
| try: | ||
| return wait.until(EC.visibility_of_element_located(locator)) | ||
| except Exception as e: | ||
| last_exception = e | ||
| raise last_exception |
There was a problem hiding this comment.
find_visible_element raises last_exception, but if locators is empty (or None), last_exception stays None and raise last_exception will itself throw a TypeError. Consider validating locators up front (e.g., raise a clear ValueError) and/or raising a deterministic exception when no locators are provided.
|
|
||
| # ------------------------------------------------------------------ | ||
| # 1. Syntax check — ast.parse catches IndentationError, SyntaxError, | ||
| # wrong boolean literals, and broken string literals. |
There was a problem hiding this comment.
The validator docstring says ast.parse catches “wrong boolean literals”, but true/false are valid identifiers in Python and will parse fine (they fail at runtime with NameError). Consider tightening the comment to reflect what ast.parse actually validates (syntax/indentation), since the JS boolean check is handled separately by the regex below.
| # wrong boolean literals, and broken string literals. | |
| # and other parse-time syntax issues such as broken string literals. |
…overage
Generated Python scripts were malformed in several ways: @classmethod landed outside the class body, indent levels drifted with each device block causing pytest to miss all but the first test function, finally was nested inside except causing an immediate SyntaxError, JS true/false appeared as Python literals, XPath selectors with double-quoted attributes broke string quoting, and multi-device if/elif/else branches were nested inside each other.
Runtime template gaps were also fixed: missing AppiumBy/By imports in test_app.py, deprecated webdriver.Remote API updated to AppiumOptions in test_base.py, missing action helpers added (send_keys_to_active_element, swipe_on_element, press_button_multiple, rotate_screen, set_location), port handling corrected in config.py, and proxy auth switched from URL-embedded credentials to an Authorization header in proxy_server.py.
A direct Python regression test (no gRPC) now generates a real zip from a 2-device fixture and runs ast.parse plus six structural checks over every .py file to protect against recurrence.
Note: the pre-existing open-handles warning from the gRPC integration tests in generate-script.test.js is unrelated and left for a separate cleanup.
Summary
Types of changes
What types of changes does your code introduce to this project? Put an
xin the boxes that apply.Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the pull request. If you are unsure about any of them, do not hesitate to ask. We are here to help! This is simply a reminder of what we are going to look for before merging your code.masterbranch of the upstream.Further comments