feat(e2e): smoketest macOS .dmg files COMPASS-8709 #6579
Conversation
| throw new Error(`${fullDestinationPath} already exists`); | ||
| } | ||
|
|
||
| exec('hdiutil', ['attach', filepath]); |
There was a problem hiding this comment.
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.
| '--workspace', | ||
| 'compass-e2e-tests', | ||
| '--', | ||
| '--test-filter=time-to-first-query', |
There was a problem hiding this comment.
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.
| @@ -0,0 +1,19 @@ | |||
| import type { SpawnSyncReturns } from 'child_process'; | |||
|
|
|||
| export function assertSpawnSyncResult( | |||
There was a problem hiding this comment.
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.
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.
ie. I basically want to happen what happens when you call a program in a shell script 😆
There was a problem hiding this comment.
I made a promisified async spawn helper rather and removed assertSpawnSyncResult. This way I can error on anything except exit code 0 and stdio gets passed through rather than accumulate all the e2e test output in the result.
| fi | ||
| npm run --workspace compass-e2e-tests smoketest | ||
| npm run --unsafe-perm --workspace compass-e2e-tests smoketest |
There was a problem hiding this comment.
"Let's see if we really need this" and then forget about it for a week 🤦
|
Closing in favor of #6582 because I made a real mess here.. |
COMPASS-8709
Installs the .app found in the .dmg to /Applications then runs the time to first query test on the app in /Applications.
To run locally: