-
Notifications
You must be signed in to change notification settings - Fork 8
[KOB-52290] Fix Python pytest generator compatibility with Appium-Python-Client 5.x #61
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
Open
mimosa767
wants to merge
8
commits into
kobiton:master
Choose a base branch
from
mimosa767:feature/KOB-52290-fix-appium-options-import
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
d65db11
[KOB-52290] Fix Python script generation and add direct integration c…
mimosa767 48ca7f0
[KOB-52290] Fix lint issues in Python generator
mimosa767 fef176a
[KOB-52290] Fix AppiumOptions import path for Appium-Python-Client 3.1.0
mimosa767 85edbe6
[KOB-52290] Fix Python proxy header forwarding and device-list respon…
mimosa767 2329fd0
fix: update AppiumOptions to UiAutomator2Options for Appium-Python-Cl…
mimosa767 6cc4c3f
fix: rewrite proxy_server.py to match Java TestNG implementation with…
mimosa767 7bd6c05
fix: update proxy_server.py to match Java TestNG implementation
mimosa767 d84c194
fix: fix concurrency, content-length, and session logging in proxy_se…
mimosa767 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,173 @@ | ||
| #!/usr/bin/env python3 | ||
| """ | ||
| Validates a generated Python pytest zip produced by PythonAppiumScriptGenerator. | ||
|
|
||
| Usage: python3 validate-python-output.py <path-to-zip> | ||
| Exit 0 = all checks passed. Exit 1 = at least one check failed (details on stdout). | ||
|
|
||
| Checks performed: | ||
| 1. ast.parse() succeeds on every .py file (catches ALL syntax/indentation errors) | ||
| 2. test_app.py imports AppiumBy and By (locator code needs both at module scope) | ||
| 3. config.py uses Python True/False (not JS true/false) | ||
| 4. config.py has @classmethod at 4-space indent (inside class body, not at col 0) | ||
| 5. test_suite.py: finally at 4-space indent (peer of try/except, not nested inside except) | ||
| 6. test_suite.py: every def test_* at module level (no leading spaces, drift check) | ||
| """ | ||
|
|
||
| import ast | ||
| import re | ||
| import sys | ||
| import zipfile | ||
|
|
||
|
|
||
| def validate(zip_path): | ||
| errors = [] | ||
|
|
||
| with zipfile.ZipFile(zip_path) as zf: | ||
| py_files = { | ||
| name: zf.read(name).decode('utf-8') | ||
| for name in zf.namelist() | ||
| if name.endswith('.py') | ||
| } | ||
|
|
||
| if not py_files: | ||
| errors.append('zip contains no .py files') | ||
| return errors | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # 1. Syntax check — ast.parse catches IndentationError, SyntaxError, | ||
| # wrong boolean literals, and broken string literals. | ||
| # ------------------------------------------------------------------ | ||
| for name, src in sorted(py_files.items()): | ||
| try: | ||
| ast.parse(src) | ||
| except SyntaxError as exc: | ||
| errors.append(f'{name}: SyntaxError line {exc.lineno}: {exc.msg}') | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # 2. test_app.py must have AppiumBy and By imported at module scope. | ||
| # Generated locator code uses these names; without the imports the | ||
| # script raises NameError at runtime before any test runs. | ||
| # ------------------------------------------------------------------ | ||
| test_app = py_files.get('test_app.py', '') | ||
| if 'from appium.webdriver.common.appiumby import AppiumBy' not in test_app: | ||
| errors.append( | ||
| 'test_app.py: missing import ' | ||
| '"from appium.webdriver.common.appiumby import AppiumBy"' | ||
| ) | ||
| if 'from selenium.webdriver.common.by import By' not in test_app: | ||
| errors.append( | ||
| 'test_app.py: missing import ' | ||
| '"from selenium.webdriver.common.by import By"' | ||
| ) | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # 2b. test_base.py must import AppiumOptions from its real module path. | ||
| # In Appium-Python-Client 3.x/4.x, AppiumOptions lives at | ||
| # appium.options.common.base, NOT appium.options — the shorter | ||
| # path raises ImportError at collection time. | ||
| # ------------------------------------------------------------------ | ||
| test_base = py_files.get('test_base.py', '') | ||
| if 'from appium.options.common.base import AppiumOptions' not in test_base: | ||
| errors.append( | ||
| 'test_base.py: missing or wrong AppiumOptions import — ' | ||
| 'must be "from appium.options.common.base import AppiumOptions"' | ||
| ) | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # 2c. test_base.py find_online_device must accept the newer | ||
| # /v1/devices response shape. test-green returns | ||
| # privateDevices/favoriteDevices/cloudDevices/etc, NOT | ||
| # deviceListData — checking only the legacy key produces | ||
| # false-negative "device not available" retries every run. | ||
| # ------------------------------------------------------------------ | ||
| if "'privateDevices'" not in test_base and 'privateDevices' not in test_base: | ||
| errors.append( | ||
| 'test_base.py: find_online_device does not recognize the newer ' | ||
| 'Kobiton /v1/devices response shape (privateDevices/cloudDevices/...). ' | ||
| 'Must union all device category keys, not only deviceListData' | ||
| ) | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # 2d. proxy_server.py must strip the client Host header before | ||
| # forwarding to Kobiton. Without this, the upstream sees | ||
| # Host: localhost:<port> and responds 404 to every request. | ||
| # ------------------------------------------------------------------ | ||
| proxy = py_files.get('proxy_server.py', '') | ||
| if "'host'" not in proxy.lower(): | ||
| errors.append( | ||
| 'proxy_server.py: does not strip the Host header before forwarding. ' | ||
| 'Upstream Kobiton routes by Host and returns 404 for localhost. ' | ||
| 'Must filter hop-by-hop/routing headers (including Host) from the forwarded request' | ||
| ) | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # 3. config.py: generated boolean capabilities must use Python | ||
| # True/False, not JavaScript true/false. | ||
| # Pattern: a dict-value position — '...': true or '...': false | ||
| # ------------------------------------------------------------------ | ||
| config = py_files.get('config.py', '') | ||
| js_bool_re = re.compile(r"^\s+'[^']+': (true|false)[,\s]*$") | ||
| for lineno, line in enumerate(config.splitlines(), 1): | ||
| if js_bool_re.match(line): | ||
| errors.append( | ||
| f'config.py line {lineno}: JS boolean literal ' | ||
| f'(must be True/False): {line.rstrip()}' | ||
| ) | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # 4. config.py: @classmethod must be indented exactly 4 spaces. | ||
| # When the first-line absorber was missing, @classmethod landed | ||
| # at column 0, placing it outside the class body. | ||
| # ------------------------------------------------------------------ | ||
| if ' @classmethod\n' not in config and not config.endswith(' @classmethod'): | ||
| errors.append( | ||
| 'config.py: no @classmethod found at 4-space indent — ' | ||
| 'generated methods are outside the class body' | ||
| ) | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # 5. test_suite.py: finally must be at 4-space indent (same level | ||
| # as try/except). The original bug nested it at 8 spaces inside | ||
| # the except block, causing an immediate SyntaxError. | ||
| # ------------------------------------------------------------------ | ||
| test_suite = py_files.get('test_suite.py', '') | ||
| for lineno, line in enumerate(test_suite.splitlines(), 1): | ||
| if line.startswith(' finally:'): | ||
| errors.append( | ||
| f'test_suite.py line {lineno}: "finally:" is at 8-space indent ' | ||
| f'(nested inside except block — must be at 4-space, peer of try/except)' | ||
| ) | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # 6. test_suite.py: every def test_* must start at column 0. | ||
| # When per-device desiredCaps methods leaked +1 indent per device, | ||
| # the second (and later) test functions shifted right and became | ||
| # nested, making pytest unable to discover them. | ||
| # ------------------------------------------------------------------ | ||
| for lineno, line in enumerate(test_suite.splitlines(), 1): | ||
| stripped = line.lstrip() | ||
| if stripped.startswith('def test_') and line != stripped: | ||
| errors.append( | ||
| f'test_suite.py line {lineno}: test function not at module level ' | ||
| f'(leading whitespace = {len(line) - len(stripped)} spaces): ' | ||
| f'{line.rstrip()}' | ||
| ) | ||
|
|
||
| return errors | ||
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| if len(sys.argv) != 2: | ||
| print('Usage: validate-python-output.py <zip_path>', file=sys.stderr) | ||
| sys.exit(2) | ||
|
|
||
| found_errors = validate(sys.argv[1]) | ||
|
|
||
| if found_errors: | ||
| for err in found_errors: | ||
| print(f'FAIL: {err}') | ||
| sys.exit(1) | ||
|
|
||
| print(f'OK: all checks passed ({sys.argv[1]})') | ||
| sys.exit(0) | ||
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,37 @@ | ||
| import path from 'path' | ||
| import fs from 'fs' | ||
| import os from 'os' | ||
| import {execFileSync} from 'child_process' | ||
| import PythonAppiumScriptGenerator from '../../src/services/python' | ||
| import {removeDir} from '../../src/utils/fs-wrapper' | ||
|
|
||
| const INPUT_FILE = path.resolve(__dirname, '../resource/python-pytest-input.json') | ||
| const VALIDATOR = path.resolve(__dirname, '../helpers/validate-python-output.py') | ||
|
|
||
| describe('PythonAppiumScriptGenerator', () => { | ||
| it('generates a valid Python pytest script from a 2-device session fixture', async () => { | ||
| const input = JSON.parse(fs.readFileSync(INPUT_FILE, 'utf-8')) | ||
| const workingDir = path.join(os.tmpdir(), `python-pytest-test-${Date.now()}`) | ||
|
|
||
| const generator = new PythonAppiumScriptGenerator({}) | ||
| const {outputFile} = await generator.run({...input, workingDir}) | ||
|
|
||
| expect(outputFile).toBeTruthy() | ||
| expect(fs.existsSync(outputFile)).toBe(true) | ||
|
|
||
| try { | ||
| execFileSync('python3', [VALIDATOR, outputFile], { | ||
| encoding: 'utf8', | ||
| stdio: 'pipe' | ||
| }) | ||
| } | ||
| catch (err) { | ||
| throw new Error( | ||
| `Python validation failed:\n${(err.stdout || '').trim()}\n${(err.stderr || '').trim()}` | ||
| ) | ||
| } | ||
| finally { | ||
| await removeDir(workingDir) | ||
| } | ||
| }, 30000) | ||
| }) |
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,180 @@ | ||
| { | ||
| "sentAt": { | ||
| "low": 1105187238, | ||
| "high": 395, | ||
| "unsigned": true | ||
| }, | ||
| "serverInfo": { | ||
| "apiUrl": "https://api.kobiton.com", | ||
| "portalUrl": "https://portal.kobiton.com", | ||
| "username": "kobitonadmin+appiumScriptGen", | ||
| "apiKey": "your_api_key" | ||
| }, | ||
| "isManualSession": true, | ||
| "manualSessionId": "605", | ||
| "devices": [ | ||
| { | ||
| "id": 1881, | ||
| "name": "A5 2020", | ||
| "capabilities": { | ||
| "platformName": "Android", | ||
| "platformVersion": "9", | ||
| "resolution": { | ||
| "width": 720, | ||
| "height": 1600, | ||
| "scale": 1 | ||
| } | ||
| } | ||
| }, | ||
| { | ||
| "id": 1882, | ||
| "name": "Galaxy S22", | ||
| "capabilities": { | ||
| "platformName": "Android", | ||
| "platformVersion": "16", | ||
| "resolution": { | ||
| "width": 1080, | ||
| "height": 2340, | ||
| "scale": 1 | ||
| } | ||
| } | ||
| } | ||
| ], | ||
| "testSteps": [ | ||
| { | ||
| "id": "17000", | ||
| "context": "NATIVE", | ||
| "selectorConfigurations": [ | ||
| { | ||
| "device": { | ||
| "deviceName": "A5 2020", | ||
| "platformVersion": "9" | ||
| }, | ||
| "selectors": [ | ||
| { | ||
| "type": "xpath", | ||
| "value": "//*[@resource-id='com.android.chrome:id/url_bar']" | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| "selectors": [ | ||
| { | ||
| "type": "xpath", | ||
| "value": "//android.widget.EditText[@resource-id='com.android.chrome:id/url_bar']" | ||
| } | ||
| ] | ||
| } | ||
| ], | ||
| "actionJson": "{\"command\":\"touchOnElement\",\"x\":\"0.387\",\"y\":\"0.430\"}", | ||
| "findingElementTimeout": 7129, | ||
| "isOnKeyboard": false | ||
| }, | ||
| { | ||
| "id": "17001", | ||
| "context": "NATIVE", | ||
| "actionJson": "{\"command\":\"sendKeys\",\"value\":\"kobiton \"}", | ||
| "findingElementTimeout": 4014, | ||
| "isOnKeyboard": false | ||
| }, | ||
| { | ||
| "id": "17002", | ||
| "context": "NATIVE", | ||
| "actionJson": "{\"command\":\"press\",\"value\":\"ENTER\"}", | ||
| "findingElementTimeout": 1612, | ||
| "isOnKeyboard": false | ||
| }, | ||
| { | ||
| "id": "17003", | ||
| "context": "WEB", | ||
| "selectorConfigurations": [ | ||
| { | ||
| "selectors": [ | ||
| { | ||
| "type": "css", | ||
| "value": "a[href='https://kobiton.com/']" | ||
| } | ||
| ] | ||
| } | ||
| ], | ||
| "actionJson": "{\"command\":\"touchOnElement\",\"x\":\"0.359\",\"y\":\"0.300\"}", | ||
| "findingElementTimeout": 8965, | ||
| "isOnKeyboard": false | ||
| }, | ||
| { | ||
| "id": "17004", | ||
| "context": "NATIVE", | ||
| "actionJson": "{\"command\":\"rotate\",\"orientation\":\"LANDSCAPE\"}", | ||
| "findingElementTimeout": 14108, | ||
| "isOnKeyboard": false | ||
| }, | ||
| { | ||
| "id": "17005", | ||
| "context": "NATIVE", | ||
| "selectorConfigurations": [ | ||
| { | ||
| "selectors": [ | ||
| { | ||
| "type": "xpath", | ||
| "value": "(//android.webkit.WebView)[1]" | ||
| } | ||
| ] | ||
| } | ||
| ], | ||
| "actionJson": "{\"command\":\"swipeFromElement\",\"x1\":\"0.506\",\"y1\":\"0.815\",\"x2\":\"0.574\",\"y2\":\"0.251\",\"duration\":637}", | ||
| "findingElementTimeout": 13081, | ||
| "isOnKeyboard": false | ||
| }, | ||
| { | ||
| "id": "17006", | ||
| "context": "NATIVE", | ||
| "actionJson": "{\"command\":\"press\",\"value\":\"BACK\"}", | ||
| "findingElementTimeout": 6526, | ||
| "isOnKeyboard": false | ||
| } | ||
| ], | ||
| "appUnderTest": { | ||
| "id": "com.android.chrome" | ||
| }, | ||
| "desiredCapabilitiesOfDevices": [ | ||
| { | ||
| "deviceId": 1881, | ||
| "desiredCapabilities": [ | ||
| {"key": "sessionName", "value": "Automation on A5 2020", "type": "string"}, | ||
| {"key": "sessionDescription", "value": "", "type": "string"}, | ||
| {"key": "deviceOrientation", "value": "portrait", "type": "string"}, | ||
| {"key": "noReset", "value": "false", "type": "bool"}, | ||
| {"key": "fullReset", "value": "true", "type": "bool"}, | ||
| {"key": "captureScreenshots", "value": "true", "type": "bool"}, | ||
| {"key": "newCommandTimeout", "value": "900", "type": "int"}, | ||
| {"key": "keepScreenOn", "value": "true", "type": "bool"}, | ||
| {"key": "deviceGroup", "value": "KOBITON", "type": "string"}, | ||
| {"key": "deviceName", "value": "A5 2020", "type": "string"}, | ||
| {"key": "platformVersion", "value": "9", "type": "string"}, | ||
| {"key": "platformName", "value": "Android", "type": "string"} | ||
| ] | ||
| }, | ||
| { | ||
| "deviceId": 1882, | ||
| "desiredCapabilities": [ | ||
| {"key": "sessionName", "value": "Automation on Galaxy S22", "type": "string"}, | ||
| {"key": "sessionDescription", "value": "", "type": "string"}, | ||
| {"key": "deviceOrientation", "value": "portrait", "type": "string"}, | ||
| {"key": "noReset", "value": "false", "type": "bool"}, | ||
| {"key": "fullReset", "value": "false", "type": "bool"}, | ||
| {"key": "captureScreenshots", "value": "true", "type": "bool"}, | ||
| {"key": "newCommandTimeout", "value": "900", "type": "int"}, | ||
| {"key": "keepScreenOn", "value": "true", "type": "bool"}, | ||
| {"key": "deviceGroup", "value": "KOBITON", "type": "string"}, | ||
| {"key": "deviceName", "value": "Galaxy S22", "type": "string"}, | ||
| {"key": "platformVersion", "value": "16", "type": "string"}, | ||
| {"key": "platformName", "value": "Android", "type": "string"} | ||
| ] | ||
| } | ||
| ], | ||
| "requestScript": { | ||
| "name": "kobiton-appium-script-s605-python-pytest", | ||
| "language": "python", | ||
| "testingFramework": "pytest" | ||
| } | ||
| } |
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.
This check still requires
test_base.pyto importAppiumOptionsfromappium.options.common.base, but the template now importsUiAutomator2Options. As written, the integration test will fail even when generation is correct. Update the assertion to match the intended options import(s) (and consider covering both Android and iOS options if supported).