-
Notifications
You must be signed in to change notification settings - Fork 8
[KOB-52290] Fix Python script generation and add direct integration c… #59
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
Closed
mimosa767
wants to merge
1
commit into
kobiton:master
from
mimosa767:feature/python-pytest-generator-fix
Closed
Changes from all commits
Commits
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,133 @@ | ||
| #!/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"' | ||
| ) | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # 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.
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.
The validator docstring says
ast.parsecatches “wrong boolean literals”, buttrue/falseare valid identifiers in Python and will parse fine (they fail at runtime withNameError). Consider tightening the comment to reflect whatast.parseactually validates (syntax/indentation), since the JS boolean check is handled separately by the regex below.