Skip to content

Commit

Permalink
Add safari browser, using AppleScript to start/stop
Browse files Browse the repository at this point in the history
  • Loading branch information
Krinkle committed Jan 6, 2025
1 parent 2dc891a commit 91c1d93
Show file tree
Hide file tree
Showing 4 changed files with 175 additions and 17 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/CI.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,7 @@ jobs:

- name: Check system browsers
run: node bin/qtap.js -v -b firefox -b chrome -b chromium -b edge test/pass.html

- name: Check system browsers (Safari)
if: ${{ runner.os == 'macOS' }}
run: node bin/qtap.js -v -b safari test/pass.html
21 changes: 16 additions & 5 deletions ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,15 +149,13 @@ Alternatives considered:
```js
// Using our utility
import qtap from 'qtap';

function myBrowser (url, signal, logger) {
async function myBrowser (url, signal, logger) {
await qtap.LocalBrowser.spawn(['/bin/mybrowser'], ['-headless', url], signal, logger );
}

// Minimal custom implementation
// Minimal sub process
import child_process from 'node:child_process';

function myBrowser (url, signal, logger) {
async function myBrowser (url, signal, logger) {
const spawned = child_process.spawn('/bin/mybrowser', ['-headless', url], { signal });
await new Promise((resolve, reject) => {
spawned.on('error', (error) => {
Expand All @@ -168,6 +166,19 @@ Alternatives considered:
});
});
}

// Minimal custom
async function myBrowser (url, signal, logger) {
// * start browser and navigate to `url`
// * if you encounter problems, throw
await new Promise((resolve, reject) => {
// * once browser has stopped, call resolve()
// * if you encounter problems, call reject()
signal.addEventListener('abort', () => {
// stop browser
});
});
}
```
* **Passing a `clientId`**.
Expand Down
152 changes: 145 additions & 7 deletions src/browsers.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,16 @@ const LocalBrowser = {
/**
* Create a new temporary directory and return its name.
*
* The newly created directory will automatically will cleaned up.
* This defaults to creating subdirectories inside Node.js `os.tmpdir`, which honors
* any TMPDIR, TMP, or TEMP environment variable.
*
* The newly created directory is automatically cleaned up at the end of the process.
*
* @returns {string}
*/
makeTempDir () {
makeTempDir (parentDir = os.tmpdir()) {
// Use mkdtemp (instead of only tmpdir) to avoid clash with past or concurrent qtap procesess.
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'qtap_'));
const dir = fs.mkdtempSync(path.join(parentDir, 'qtap_'));
tempDirs.push(dir);
return dir;
},
Expand Down Expand Up @@ -279,22 +282,157 @@ async function chromium (paths, url, signal, logger) {
'--disable-gpu',
'--disable-dev-shm-usage'
]),
...(process.env.CHROMIUM_FLAGS ? process.env.CHROMIUM_FLAGS.split(/\s+/) : []),
...(process.env.CHROMIUM_FLAGS ? process.env.CHROMIUM_FLAGS.split(/\s+/) : (
process.env.CI ? ['--no-sandbox'] : [])
),
url
];
await LocalBrowser.spawn(paths, args, signal, logger);
}

/**
* Known approaches:
*
* - `Safari <file>`. This does not allow URLs. Safari allows only local files to be passed.
*
* - `Safari redirect.html`, without other arguments, worked from 2012-2018, as used by Karma.
* This "trampoline" approach involves creating a temporary HTML file
* with `<script>window.location='<url>';</script>`, which we open instead.
* https://github.com/karma-runner/karma-safari-launcher/blob/v1.0.0/index.js
* https://github.com/karma-runner/karma/blob/v0.3.5/lib/launcher.js#L213
* https://github.com/karma-runner/karma/commit/5513fd66ae
*
* This is no longer viable after macOS 10.14 Mojave, because macOS SIP prompts the user
* due to our temporary file being outside `~/Library/Containers/com.apple.Safari`.
* https://github.com/karma-runner/karma-safari-launcher/issues/29
*
* - `open -F -W -n -b com.apple.Safari <url>`. This starts correctly, but doesn't expose
* a PID to cleanly end the process.
* https://github.com/karma-runner/karma-safari-launcher/issues/29
*
* - `Safari container/redirect.html`. macOS SIP denies this by default for the same reason.
* But, as long as you grant an exemption to Terminal to write to Safari's container, or
* grant it Full Disk Access, this is viable.
* https://github.com/flutter/engine/pull/27567
* https://github.com/marcoscaceres/karma-safaritechpreview-launcher/issues/7
*
* It seems that GitHub CI has pre-approved limited access in its macOS images, to make
* this work [1][2]. This might be viable if it is tolerable to prompt on first local use,
* and require granting said access to the Terminal in general (which has lasting
* consequences beyond QTap).
*
* - native app Swift/ObjectiveC proxy. This reportedly works but requires
* a binary which requires compilation and makes auditing significantly harder.
* https://github.com/karma-runner/karma-safari-launcher/issues/29
* https://github.com/muthu90ec/karma-safarinative-launcher/
*
* - `osascript -e <script>`
* As of macOS 13 Ventura (or earlier?), this results in a prompt for
* "Terminal wants access to control Safari", from which osascript will eventually
* timeout and report "Safari got an error: AppleEvent timed out".
*
* While past discussions suggest that GitHub CI has this pre-approved [1][2],
* as of writing in Jan 2025 with macOS 13 images, this approval does not include
* access from Terminal to Safari, thus causing the same "AppleEvent timed out".
*
* https://github.com/brandonocasey/karma-safari-applescript-launcher
* https://github.com/brandonocasey/karma-safari-applescript-launcher/issues/5
*
* - `osascript MyScript.scpt`. This avoids the need for quote escaping in the URL, by
* injecting it properly as a parameter instead. Used by Google's karma-webkit-launcher
* https://github.com/google/karma-webkit-launcher/commit/31a2ad8037
*
* - `safaridriver -p <port>`, and then make an HTTP request to create a session,
* navigate the session, and to delete the session. This addresses all the concerns,
* and seems to be the best as of 2025. The only downside is that it requires a bit
* more code (available port, and HTTP requests).
* https://github.com/flutter/engine/pull/33757
*
* See also:
* - Unresolved as of writing, https://github.com/testem/testem/issues/1387
* - Unresolved as of writing, https://github.com/emberjs/data/issues/7170
*
* [1]: https://github.com/actions/runner-images/issues/4201
* [2]: https://github.com/actions/runner-images/issues/7531
*/
const safariDriverPort = null;
async function safari (url, signal, logger) {
// Support overriding to "/Applications/Safari Technology Preview.app/Contents/MacOS/safaridriver"
// https://developer.apple.com/documentation/webkit/testing-with-webdriver-in-safari
const safaridriverBin = process.env.SAFARIDRIVER_BIN || which.sync('safaridriver', { nothrow: true });
if (process.platform !== 'darwin' || !safaridriverBin) {
throw new Error('Safari requires macOS and safaridriver');
}

// TODO: De-duplicate via safariDriverPort so that only one is running
async function findAvailablePort() {
const net = await import('node:net');
return new Promise((resolve, reject) => {
const srv = net.createServer();
srv.listen(0, () => {
const port = srv.address().port;
srv.close(() => resolve(port));
});
})
}

const port = await findAvailablePort();
const spawned = LocalBrowser.spawn(safaridriverBin, ['-p', port], signal, logger);

// https://developer.apple.com/documentation/webkit/macos-webdriver-commands-for-safari-12-and-later
async function webdriverReq(method, endpoint, body) {
const resp = await fetch(`http://127.0.0.1:${port}${endpoint}`, {
method: method,
body: JSON.stringify(body)
});
const data = await resp.json();
if (!resp.ok) {
throw `HTTP ${resp.status} ${data?.value?.error}, ${data?.value?.message || ''}`;
}
return data.value;
}

// Since Node.js 18, connecting to "localhost" favours IPv6 (::1), whereas safaridriver
// listens exclusively on IPv4 (127.0.0.1). This was fixed in Node.js 20 by trying both.
// https://github.com/nodejs/node/issues/40702
// https://github.com/nodejs/node/pull/44731
// https://github.com/node-fetch/node-fetch/issues/1624
let sessionId;
for (let i = 1; i <= 10; i++) {
try {
const session = await webdriverReq('POST', '/session/', { capabilities: { browserName: 'safari' }});
sessionId = session.sessionId;
break; // Connected
} catch (e) {
if (e && (e.code === 'ECONNREFUSED' || (e.cause && e.cause.code === 'ECONNREFUSED'))) {
// Wait another 10ms-100ms for safaridriver to start. We'll wait ~550ms in total.
logger.debug('safaridriver_waiting', `Attempt #${i}: ${e.code || e.cause.code}. Try again in ${i * 10}ms.`);
await new Promise(resolve => setTimeout(resolve, i * 10));
} else {
logger.warning('safaridriver_session_error', e);
throw new Error('Failed to create new session');
}
}
}

try {
await webdriverReq('POST', `/session/${sessionId}/url`, { url: url });
} catch (e) {
logger.warning('safaridriver_url_error', e);
throw new Error('Failed to create new tab');
}

await spawned;
}

export default {
LocalBrowser,

firefox,
chrome: chromium.bind(null, concatGenFn(getChromePaths, getChromiumPaths, getEdgePaths)),
chromium: chromium.bind(null, concatGenFn(getChromiumPaths, getChromePaths, getEdgePaths)),
edge: chromium.bind(null, concatGenFn(getEdgePaths)),

//
// TODO: safari: [],
safari,

// TODO: browserstack
// - browserstack/firefox_45
Expand Down
15 changes: 10 additions & 5 deletions src/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,21 +137,21 @@ class ControlServer {
};

const tapFinishFinder = tapFinished({ wait: 0 }, () => {
logger.debug('browser_tap_finished', 'Requesting browser stop');
logger.debug('browser_tap_finished', 'Test has finished, stopping browser');

stopBrowser('QTap: browser_tap_finished');
});

const tapParser = new TapParser();
tapParser.on('bailout', (reason) => {
logger.warning('browser_tap_bailout', `Test ended unexpectedly, stopping browser. Reason: ${reason}`);
summary.ok = false;
logger.debug('browser_tap_bailout', reason);

stopBrowser('QTap: browser_tap_bailout');
});
tapParser.once('fail', () => {
logger.debug('browser_tap_fail', 'Results indicate at least one test has failed assertions');
summary.ok = false;
logger.debug('browser_tap_fail', 'One or more tests failed');
});
// Debugging
// tapParser.on('assert', logger.debug.bind(logger, 'browser_tap_assert'));
Expand All @@ -176,7 +176,7 @@ class ControlServer {
// creating tons of timers when processing a large batch of test results back-to-back.
clientIdleTimer = setTimeout(function qtapClientTimeout () {
if ((performance.now() - browser.clientIdleActive) > CLIENT_IDLE_TIMEOUT) {
logger.debug('browser_idle_timeout', 'Requesting browser stop');
logger.warning('browser_idle_timeout', `Browser timed out after ${CLIENT_IDLE_TIMEOUT}ms, stopping browser`);
// TODO:
// Produce a tap line to report this test failure to CLI output/reporters.
summary.ok = false;
Expand Down Expand Up @@ -318,7 +318,12 @@ class ControlServer {

const clientId = url.searchParams.get('qtap_clientId');
if (url.pathname === '/' && clientId !== null) {
this.logger.debug('respond_static_testfile', clientId);
const browser = this.browsers.get(clientId);
if (browser) {
browser.logger.debug('browser_connected', 'Browser connected! Serving test file.');
} else {
this.logger.debug('respond_static_testfile', clientId);
}
resp.writeHead(200, { 'Content-Type': MIME_TYPES[ext] || MIME_TYPES.html });
resp.write(await this.getTestFile(clientId));
resp.end();
Expand Down

0 comments on commit 91c1d93

Please sign in to comment.