Skip to content

Enhance Appium integration and screenshot handling #424

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.

Sign up for GitHub

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
hawkeyexl wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Enhance Appium integration and screenshot handling #424

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
858e55b
Enhance Appium integration and testing
hawkeyexl Jan 25, 2026
2ea99e9
Clamp screenshot coordinates to viewport bounds and ensure minimum di…
hawkeyexl Jan 25, 2026
9451ff5
Fix test failures from incorrect parameter and outdated selectors (#425)
Copilot Jan 26, 2026
e007ce6
Add navigation test for local server using runStep
hawkeyexl Jan 26, 2026
c60dbd9
Enhance port availability checks and error reporting
hawkeyexl Jan 26, 2026
c779ea7
Revert "Enhance port availability checks and error reporting"
hawkeyexl Jan 26, 2026
39be475
Address CodeRabbit review comments: improve port error handling, add …
hawkeyexl Jan 26, 2026
6ad9590
Fix CodeRabbit comments: axios timeout overshoot, dual-host pre-check…
hawkeyexl Jan 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 134 additions & 35 deletions src/tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const kill = require("tree-kill");
const wdio = require("webdriverio");
const os = require("os");
const net = require("net");
const { log, replaceEnvs } = require("./utils");
const axios = require("axios");
const { instantiateCursor } = require("./tests/moveTo");
Expand Down Expand Up @@ -31,6 +32,7 @@ const { uploadChangedFiles } = require("./integrations");
exports.runSpecs = runSpecs;
exports.runViaApi = runViaApi;
exports.getRunner = getRunner;
exports.checkPortAvailable = checkPortAvailable;
// exports.appiumStart = appiumStart;
// exports.appiumIsReady = appiumIsReady;
// exports.driverStart = driverStart;
Expand Down Expand Up @@ -380,6 +382,7 @@ async function runSpecs({ resolvedTests }) {
const availableApps = runnerDetails.availableApps;
const metaValues = { specs: {} };
let appium;
let appiumHost;
const report = {
summary: {
specs: {
Expand Down Expand Up @@ -429,8 +432,14 @@ async function runSpecs({ resolvedTests }) {
appium.stderr.on("data", (data) => {
// console.error(`stderr: ${data}`);
});
await appiumIsReady();
log(config, "debug", "Appium is ready.");
try {
appiumHost = await appiumIsReady();
log(config, "debug", `Appium is ready on ${appiumHost}.`);
} catch (error) {
// Clean up Appium process on timeout/failure
kill(appium.pid);
throw error;
}
}

// Iterate specs
Expand Down Expand Up @@ -541,7 +550,7 @@ async function runSpecs({ resolvedTests }) {

// Instantiate driver
try {
driver = await driverStart(caps);
driver = await driverStart(caps, appiumHost);
} catch (error) {
try {
// If driver fails to start, try again as headless
Expand All @@ -552,15 +561,15 @@ async function runSpecs({ resolvedTests }) {
);
context.browser.headless = true;
caps = getDriverCapabilities({
config: config,
runnerDetails: runnerDetails,
name: context.browser.name,
options: {
width: context.browser?.window?.width || 1200,
height: context.browser?.window?.height || 800,
headless: context.browser?.headless !== false,
},
});
driver = await driverStart(caps);
driver = await driverStart(caps, appiumHost);
} catch (error) {
let errorMessage = `Failed to start context '${context.browser?.name}' on '${platform}'.`;
if (context.browser?.name === "safari")
Expand Down Expand Up @@ -939,36 +948,119 @@ async function runStep({
return actionResult;
}

// Delay execution until Appium server is available.
async function appiumIsReady() {
let isReady = false;
while (!isReady) {
/**
* Check if a port is available (not in use).
* @param {number} port - Port number to check
* @param {string} host - Host to check on (default: 127.0.0.1)
* @returns {Promise<boolean>} - True if port is available, false if in use
*/
async function checkPortAvailable(port, host = "127.0.0.1") {
return new Promise((resolve) => {
const server = net.createServer();
server.once("error", (err) => {
if (err.code === "EADDRINUSE") {
resolve(false); // Port is in use
} else {
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

The checkPortAvailable function has a potential resource leak. If an unknown error occurs (line 986), the function resolves with true but never closes the server, which could leave a socket open. Consider adding server.close() before resolving in the error handler for unknown errors, or handle server cleanup more systematically.

Suggested change
} else {
} else {
server.close();

Copilot uses AI. Check for mistakes.
resolve(true); // Other error, assume available
}
});
server.once("listening", () => {
server.close();
resolve(true); // Port is available
});
server.listen(port, host);
});
}

/**
* Delay execution until Appium server is available.
* Uses 127.0.0.1 with localhost fallback for cross-platform compatibility.
* @param {number} timeoutMs - Maximum time to wait in milliseconds (default: 60000)
* @returns {Promise<string>} - The hostname that worked (127.0.0.1 or localhost)
* @throws {Error} - If Appium fails to start within timeout
*/
async function appiumIsReady(timeoutMs = 60000) {
const startTime = Date.now();
const hosts = ["127.0.0.1", "localhost"];
let lastError = null;
let successHost = null;

while (Date.now() - startTime < timeoutMs) {
// Retry delay
// TODO: Add configurable retry delay
// TODO: Add configurable timeout duration
await new Promise((resolve) => setTimeout(resolve, 1000));
try {
let resp = await axios.get("http://0.0.0.0:4723/status");
if (resp.status === 200) isReady = true;
} catch {}

for (const host of hosts) {
try {
const resp = await axios.get(`http://${host}:4723/status`, {
timeout: 5000, // 5 second request timeout
});
Comment on lines 1024 to 1027
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 26, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Clamp axios timeout to the remaining budget (avoid minimum 100ms).

Math.max(100, hostRemaining) can overshoot the deadline, which the new tests try to enforce strictly. If the remaining budget is small, prefer using it as-is (or skip the request).

🛠️ Suggested fix 
-        const resp = await axios.get(`http://${host}:4723/status`, {
-          timeout: Math.min(5000, Math.max(100, hostRemaining)), // Clamp to remaining time
-        });
+        const resp = await axios.get(`http://${host}:4723/status`, {
+          timeout: Math.min(5000, hostRemaining), // Clamp to remaining time
+        });
🤖 Prompt for AI Agents 
In `@src/tests.js` around lines 1009 - 1012, The axios timeout clamp currently
uses Math.min(5000, Math.max(100, hostRemaining)) which forces a 100ms minimum
and can overshoot the overall deadline; update the try block where axios.get is
called so it uses a timeout of Math.min(5000, hostRemaining) and before making
the request skip/return if hostRemaining <= 0 (or otherwise treat very small
remaining budgets as a skip) to avoid exceeding the deadline; locate and modify
the axios.get call in src/tests.js (the try block that constructs the timeout)
to implement this change.

if (resp.status === 200) {
successHost = host;
return successHost;
}
} catch (err) {
lastError = err;
// Continue to next host or retry
}
}
}

// Timeout reached - build descriptive error message
const elapsed = Date.now() - startTime;
const portAvailable = await checkPortAvailable(4723);
const platform = process.platform;

let errorMsg = `Appium failed to start within ${Math.round(elapsed / 1000)} seconds.\n`;
errorMsg += `Platform: ${platform}\n`;
errorMsg += `Port 4723 status: ${portAvailable ? "available (not bound)" : "in use (bound)"}\n`;

if (lastError) {
errorMsg += `Last connection error: ${lastError.message}\n`;
}
return isReady;

if (platform === "win32") {
errorMsg += "\nWindows troubleshooting:\n";
errorMsg += "- Check Windows Firewall settings for port 4723\n";
errorMsg += "- Temporarily disable antivirus software\n";
errorMsg += "- Run as Administrator if port binding fails\n";
} else if (platform === "darwin") {
errorMsg += "\nmacOS troubleshooting:\n";
errorMsg += "- Check System Preferences > Security & Privacy > Firewall\n";
errorMsg += "- Ensure no VPN is blocking localhost connections\n";
}

throw new Error(errorMsg);
}

// Start the Appium driver specified in `capabilities`.
async function driverStart(capabilities) {
const driver = await wdio.remote({
protocol: "http",
hostname: "0.0.0.0",
port: 4723,
path: "/",
logLevel: "error",
capabilities,
connectionRetryTimeout: 600000, // 10 minutes
waitforTimeout: 600000, // 10 minutes
});
driver.state = { url: "", x: null, y: null };
return driver;
// Uses hostname determined by appiumIsReady() for cross-platform compatibility.
async function driverStart(capabilities, hostname = "127.0.0.1") {
const hosts = hostname ? [hostname] : ["127.0.0.1", "localhost"];
let lastError = null;

for (const host of hosts) {
try {
const driver = await wdio.remote({
protocol: "http",
hostname: host,
port: 4723,
path: "/",
logLevel: "error",
capabilities,
connectionRetryTimeout: 60000, // 60 seconds (reduced from 10 minutes)
waitforTimeout: 60000, // 60 seconds (reduced from 10 minutes)
});
driver.state = { url: "", x: null, y: null };
return driver;
} catch (err) {
lastError = err;
// Try next hostname
}
}

throw new Error(
`Failed to connect to Appium driver on hosts [${hosts.join(", ")}]: ${lastError?.message || "Unknown error"}`
);
}

/**
Expand Down Expand Up @@ -1029,9 +1121,16 @@ async function getRunner(options = {}) {
cwd: path.join(__dirname, ".."),
});

// Wait for Appium to be ready
await appiumIsReady();
log(config, "debug", "Appium is ready for external driver.");
// Wait for Appium to be ready and get the working hostname
let appiumHost;
try {
appiumHost = await appiumIsReady();
log(config, "debug", `Appium is ready on ${appiumHost} for external driver.`);
} catch (error) {
// Clean up Appium process on timeout/failure
kill(appium.pid);
throw error;
}

// Get Chrome driver capabilities
const caps = getDriverCapabilities({
Expand All @@ -1044,10 +1143,10 @@ async function getRunner(options = {}) {
},
});

// Start the runner
// Start the runner using the hostname that worked for Appium status check
let runner;
try {
runner = await driverStart(caps);
runner = await driverStart(caps, appiumHost);
} catch (error) {
// If runner fails, attempt to set headless and retry
try {
Expand All @@ -1057,7 +1156,7 @@ async function getRunner(options = {}) {
"Failed to start Chrome runner. Retrying as headless."
);
caps["goog:chromeOptions"].args.push("--headless", "--disable-gpu");
runner = await driverStart(caps);
runner = await driverStart(caps, appiumHost);
} catch (error) {
// If runner fails, clean up Appium and rethrow
kill(appium.pid);
Expand Down
26 changes: 26 additions & 0 deletions src/tests/saveScreenshot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,32 @@ async function saveScreenshot({ config, step, driver }) {
rect.width += padding.left + padding.right;
rect.height += padding.top + padding.bottom;

// Get viewport dimensions for clamping
const viewport = await driver.execute(() => ({
width: window.innerWidth,
height: window.innerHeight,
}));

// Clamp coordinates to viewport bounds (prevent negative values and overflow)
if (rect.x < 0) {
rect.width += rect.x; // Reduce width by the amount we're clamping
rect.x = 0;
}
if (rect.y < 0) {
rect.height += rect.y; // Reduce height by the amount we're clamping
rect.y = 0;
}
if (rect.x + rect.width > viewport.width) {
rect.width = viewport.width - rect.x;
}
if (rect.y + rect.height > viewport.height) {
rect.height = viewport.height - rect.y;
}

Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

The clamping logic may produce invalid dimensions in edge cases. If an element with padding extends significantly beyond the viewport bounds on multiple sides, the sequential clamping operations (lines 247-260) could result in rect.width or rect.height becoming zero or negative before the Math.max(1, ...) calls at lines 263-264. For example, if rect.x = -100, rect.width = 50, after line 248, rect.width = -50. While lines 263-264 ensure minimum 1 pixel, this may not represent the intended crop area. Consider validating that the element is at least partially visible in the viewport before attempting to crop.

Suggested change
// After clamping, ensure the element is at least partially visible and the rect is valid
const isNonPositiveSize = rect.width <= 0 || rect.height <= 0;
const isCompletelyOutsideViewport =
rect.x >= viewport.width ||
rect.y >= viewport.height ||
rect.x + rect.width <= 0 ||
rect.y + rect.height <= 0;
if (isNonPositiveSize || isCompletelyOutsideViewport) {
throw new Error(
"Cannot capture screenshot: target element is not sufficiently visible within the viewport."
);
}

Copilot uses AI. Check for mistakes.
// Ensure width and height are at least 1 pixel
rect.width = Math.max(1, rect.width);
rect.height = Math.max(1, rect.height);

// Scale the values based on the pixel density
rect.x *= pixelDensity;
rect.y *= pixelDensity;
Expand Down
2 changes: 1 addition & 1 deletion test/artifacts/find_rightClick.spec.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
},
{
"action": "find",
"selector": "#searchbox_input",
"selector": "input[name=\"q\"]",
"click": {
"button": "right"
}
Expand Down
6 changes: 3 additions & 3 deletions test/artifacts/screenshot.spec.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"path": "crop.png",
"directory": "static/images",
"maxVariation": 0.1,
"crop": "#searchbox_input"
"crop": "input[name=\"q\"]"
}
},
{
Expand All @@ -30,7 +30,7 @@
"directory": "static/images",
"maxVariation": 0.1,
"crop": {
"selector": "#searchbox_input",
"selector": "input[name=\"q\"]",
"padding": 5
}
}
Expand All @@ -42,7 +42,7 @@
"directory": "static/images",
"maxVariation": 0.1,
"crop": {
"selector": "#searchbox_input",
"selector": "input[name=\"q\"]",
"padding": {
"top": 5,
"right": 5,
Expand Down
Loading