-
Notifications
You must be signed in to change notification settings - Fork 188
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
feat(e2e): smoketest macOS .dmg files COMPASS-8709 #6579
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import type { SpawnSyncReturns } from 'child_process'; | ||
|
||
export function assertSpawnSyncResult( | ||
result: SpawnSyncReturns<string>, | ||
name: string | ||
) { | ||
if (result.status === null) { | ||
if (result.signal !== null) { | ||
throw new Error(`${name} terminated due to signal ${result.signal}`); | ||
} | ||
|
||
// not supposed to be possible to get here, but just in case | ||
throw new Error(`${name} terminated with no status or signal`); | ||
} | ||
|
||
if (result.status !== 0) { | ||
throw new Error(`${name} failed with exit code ${result.status}`); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
import { existsSync } from 'fs'; | ||
import { assertSpawnSyncResult } from './helpers'; | ||
import type { InstalledAppInfo, Package } from './types'; | ||
import { spawnSync } from 'child_process'; | ||
|
||
function exec(command: string, args: string[]) { | ||
console.log(command, ...args); | ||
|
||
assertSpawnSyncResult( | ||
spawnSync(command, args, { | ||
encoding: 'utf8', | ||
stdio: 'inherit', | ||
}), | ||
`${command} ${args.join(' ')}` | ||
); | ||
} | ||
|
||
export async function installMacDMG( | ||
appName: string, | ||
{ filepath }: Package | ||
): Promise<InstalledAppInfo> { | ||
const fullDestinationPath = `/Applications/${appName}.app`; | ||
|
||
if (existsSync(fullDestinationPath)) { | ||
throw new Error(`${fullDestinationPath} already exists`); | ||
} | ||
|
||
exec('hdiutil', ['attach', filepath]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm in two minds about whether this should just be a shell script at this point, but I suppose it is easy to execute a shell script instead at a later stage. I suppose we'll see what happens once we get to the windows setup .exe and .msi what works best there. For .tar.gz and .zip files it would likely just be one line anyway. |
||
try { | ||
exec('cp', ['-r', `/Volumes/${appName}/${appName}.app`, '/Applications']); | ||
} finally { | ||
exec('hdiutil', ['detach', `/Volumes/${appName}`]); | ||
} | ||
|
||
return Promise.resolve({ | ||
appName, | ||
appPath: `/Applications/${appName}.app`, | ||
}); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
export type Package = { | ||
filename: string; | ||
filepath: string; | ||
}; | ||
|
||
export type InstalledAppInfo = { | ||
appName: string; | ||
appPath: string; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
#!/usr/bin/env npx ts-node | ||
import { spawnSync } from 'child_process'; | ||
import { createWriteStream, existsSync, promises as fs } from 'fs'; | ||
import path from 'path'; | ||
import yargs from 'yargs'; | ||
|
@@ -7,6 +8,9 @@ import { hideBin } from 'yargs/helpers'; | |
import https from 'https'; | ||
import { pick } from 'lodash'; | ||
import { handler as writeBuildInfo } from 'hadron-build/commands/info'; | ||
import type { InstalledAppInfo, Package } from './installers/types'; | ||
import { installMacDMG } from './installers/mac-dmg'; | ||
import { assertSpawnSyncResult } from './installers/helpers'; | ||
|
||
const argv = yargs(hideBin(process.argv)) | ||
.scriptName('smoke-tests') | ||
|
@@ -137,6 +141,10 @@ async function run() { | |
writeBuildInfo(infoArgs); | ||
const buildInfo = JSON.parse(await fs.readFile(infoArgs.out, 'utf8')); | ||
|
||
if (!buildInfoIsCommon(buildInfo)) { | ||
throw new Error('buildInfo is missing'); | ||
} | ||
|
||
// filter the extensions given the platform (isWindows, isOSX, isUbuntu, isRHEL) and extension | ||
const { isWindows, isOSX, isRHEL, isUbuntu, extension } = context; | ||
|
||
|
@@ -150,9 +158,9 @@ async function run() { | |
|
||
if (!context.skipDownload) { | ||
await Promise.all( | ||
packages.map(async ({ name, filepath }) => { | ||
packages.map(async ({ filename, filepath }) => { | ||
await fs.mkdir(path.dirname(filepath), { recursive: true }); | ||
const url = `https://${context.bucketName}.s3.amazonaws.com/${context.bucketKeyPrefix}/${name}`; | ||
const url = `https://${context.bucketName}.s3.amazonaws.com/${context.bucketKeyPrefix}/${filename}`; | ||
console.log(url); | ||
return downloadFile(url, filepath); | ||
}) | ||
|
@@ -162,6 +170,24 @@ async function run() { | |
verifyPackagesExist(packages); | ||
|
||
// TODO(COMPASS-8533): extract or install each package and then test the Compass binary | ||
for (const pkg of packages) { | ||
let appInfo: InstalledAppInfo | undefined = undefined; | ||
|
||
console.log('installing', pkg.filepath); | ||
|
||
if (pkg.filename.endsWith('.dmg')) { | ||
appInfo = await installMacDMG(buildInfo.productName, pkg); | ||
} | ||
|
||
// TODO: all the other installers go here | ||
|
||
if (appInfo) { | ||
console.log('testing', appInfo.appPath); | ||
testInstalledApp(appInfo); | ||
} else { | ||
console.log(`no app got installed for ${pkg.filename}`); | ||
} | ||
} | ||
} | ||
|
||
function platformFromContext( | ||
|
@@ -189,6 +215,18 @@ type PackageFilterConfig = Pick< | |
|
||
// subsets of the hadron-build info result | ||
|
||
const commonKeys = ['productName']; | ||
type CommonBuildInfo = Record<typeof commonKeys[number], string>; | ||
|
||
function buildInfoIsCommon(buildInfo: any): buildInfo is CommonBuildInfo { | ||
for (const key of commonKeys) { | ||
if (!buildInfo[key]) { | ||
return false; | ||
} | ||
} | ||
return true; | ||
} | ||
|
||
const windowsFilenameKeys = [ | ||
'windows_setup_filename', | ||
'windows_msi_filename', | ||
|
@@ -245,11 +283,6 @@ function buildInfoIsRHEL(buildInfo: any): buildInfo is RHELBuildInfo { | |
return true; | ||
} | ||
|
||
type Package = { | ||
name: string; | ||
filepath: string; | ||
}; | ||
|
||
function getFilteredPackages( | ||
compassDir: string, | ||
buildInfo: any, | ||
|
@@ -282,11 +315,11 @@ function getFilteredPackages( | |
const extension = config.extension; | ||
|
||
return names | ||
.filter((name) => !extension || name.endsWith(extension)) | ||
.map((name) => { | ||
.filter((filename) => !extension || filename.endsWith(extension)) | ||
.map((filename) => { | ||
return { | ||
name, | ||
filepath: path.join(compassDir, 'dist', name), | ||
filename, | ||
filepath: path.join(compassDir, 'dist', filename), | ||
}; | ||
}); | ||
} | ||
|
@@ -333,6 +366,32 @@ function verifyPackagesExist(packages: Package[]): void { | |
} | ||
} | ||
|
||
function testInstalledApp(appInfo: InstalledAppInfo) { | ||
const result = spawnSync( | ||
'npm', | ||
[ | ||
'run', | ||
'--unsafe-perm', | ||
'test-packaged', | ||
'--workspace', | ||
'compass-e2e-tests', | ||
'--', | ||
'--test-filter=time-to-first-query', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The filter will probably change once we add tests that test auto-update since it is easiest to test that by running the app inside an e2e test so we can use webdriverio. But this is good enough for now because it tests that the app starts up and can execute a query. |
||
], | ||
{ | ||
encoding: 'utf8', | ||
stdio: 'inherit', | ||
env: { | ||
...process.env, | ||
COMPASS_APP_NAME: appInfo.appName, | ||
COMPASS_APP_PATH: appInfo.appPath, | ||
}, | ||
} | ||
); | ||
|
||
assertSpawnSyncResult(result, 'npm run test-packaged'); | ||
} | ||
|
||
run() | ||
.then(function () { | ||
console.log('done'); | ||
|
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.
Non-blocking, and no need to change anything in this PR, but:
The typical best practice in Node.js in general is to use
execFile
(and usually promisified so it's not completely unworkable).spawnSync
is obviously also fine to use out of convenience in tests, since there's no need to care about performance or limitations of the synchronous method as much, but I'd be wary of building an entire test helper system around it – if you're at the point where you're introducing helpers already, you might as well just stick to the async method. (Again, this all matters only to a limited extent, but "follow best practices for production code even in tests" just makes it easier to also follow best practices in actual production code)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.
Regarding sync vs async: I was on the fence and I'll just change it to async.
Regarding exec vs execFile vs spawn vs spawn: I have no idea when to use which. I suppose exec() at least has a callback for when it finishes, but I got stuck on the fact that in a general sense I don't want all the stdout and stderr in a variable. In these cases there's little output, but in a more general sense you can never be too sure. To me the ideal seems to pass it through to stdout and stderr and just resolve a promise with an error. None of them seem to do that or close to it out of the box.
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.
ie. I basically want to happen what happens when you call a program in a shell script 😆