test: giving steroids to the test suite 💪 #122
Conversation
mmaietta
force-pushed
the
snapshot-files-2
branch
from
65805ef to
b8ccf89
Compare
test/util.ts
Outdated
| @@ -0,0 +1,179 @@ | |||
| import { readArchiveHeaderSync } from '@electron/asar/lib/disk'; | |||
There was a problem hiding this comment.
We shouldn't reach into @electron/asar internals here. Looks like this is already exposed via the public API as getRawHeader, and that should handle the typings for us as well.
There was a problem hiding this comment.
Thank you for the feedback! Updated the PR to use the exposed public method
Remove warnings by adding transform regex to `ts-jest` and `testMatch`.
There was a problem hiding this comment.
I believe the changes in this file can also be dropped now that the typings directory has been removed from the PR.
There was a problem hiding this comment.
Oh derp, my apologies. Yep! I will push the change.
test/util.ts
Outdated
| !path.basename(p).endsWith('.asar') && | ||
| path.basename(p).includes('app') && // it's an app dir | ||
| !p.endsWith('.app'), // but we don't want any sub-apps (Something.app) |
There was a problem hiding this comment.
Make the logic here clearer.
| !path.basename(p).endsWith('.asar') && | |
| path.basename(p).includes('app') && // it's an app dir | |
| !p.endsWith('.app'), // but we don't want any sub-apps (Something.app) | |
| ['app', 'app.asar.unpacked'].includes(path.basename(p)) |
There was a problem hiding this comment.
I think that would exclude shimmed app directories, right? We should also include app-x64.asar.unpacked and app-arm64.asar.unpacked? I can push a commit updating the logic
There was a problem hiding this comment.
Oh. Good catch!
path.basename(p) === 'app' || path.basename(p).endsWith('.asar.unpacked')
There was a problem hiding this comment.
Ahh, just realized from the test snapshots, it also needs to include app-x64 and app-arm64 to include shimmed no-asar universal apps so that there's coverage for test case should shim two different app folders
Pushing a change:
// check all app and unpacked dirs (incl. shimmed)
const dirsToSnapshot = [
'app',
'app.asar.unpacked',
'app-x64',
'app-x64.asar.unpacked',
'app-arm64',
'app-arm64.asar.unpacked',
];
const appDirs = resourcesDirContents
.filter((p) => dirsToSnapshot.includes(path.basename(p)))
.sort();
test/util.ts
Outdated
| .filter( | ||
| (appFile) => | ||
| appFile.type === fileUtils.AppFileType.INFO_PLIST && | ||
| !appFile.relativePath.includes('Framework'), |
There was a problem hiding this comment.
Is this a spelling mistake? Maybe it should be framework or Frameworks.
There was a problem hiding this comment.
This was intentional for filtering out Framework, but looks like .framework and Frameworks would also work. Which would you prefer?
/test/fixtures/apps/Arm64-1.app
└── Contents
├── Frameworks
│ ├── Electron Framework.framework
│ │ ├── Electron Framework -> Versions/Current/Electron Framework
│ │ ├── Helpers -> Versions/Current/Helpers
│ │ ├── Libraries -> Versions/Current/Libraries
│ │ ├── Resources -> Versions/Current/Resources
│ │ └── Versions
│ │ ├── A
│ │ │ ├── Electron Framework
│ │ │ ├── Helpers
│ │ │ │ └── chrome_crashpad_handler
│ │ │ ├── Libraries
│ │ │ │ ├── libEGL.dylib
│ │ │ │ ├── libGLESv2.dylib
│ │ │ │ ├── libffmpeg.dylib
│ │ │ │ ├── libvk_swiftshader.dylib
│ │ │ │ └── vk_swiftshader_icd.json
│ │ │ └── Resources
│ │ │ ├── Info.plist
There was a problem hiding this comment.
It seems that Frameworks is the best option?
To make the logic here stronger and clearer, maybe !appFile.relativePath.includes(path.join('Contents', 'Frameworks')) would be better?
|
🎉 This PR is included in version 2.0.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
3 participants
This is purely a test suite upgrade except for code change:
asar.listPackagewas missing a 2nd argument. I checked withelectron/asarand its default behavior isisPack: false, so functionally it is the same.Purpose:
There are some bugs I'd like to fix (#117 #116), but I wanted to beef up the test suite to be more robust first to prep for the fix PRs.
I added some fun helper test utils and resolved 2
it.todotest cases.Test Cases added:
should not inject ElectronAsarIntegrity into 'infoPlistsToIgnore'should shim two different app foldersFunctionally, this PR adds:
verifyApphelper function does all the below:ensureUniversal(consolidating existing logic)verifyHeader- parses out unstable properties ("offset" varies per build machine?) and stores a snapshotverifyFileTree- Traverses forappdirs (both merged and shimmed) and snapshots the tree hierarchy and plain text files (also used for unpacked dir snapshots)extractAsarIntegrity- a loop verifies theElectronAsarIntegrityentry in theInfo.plistin all non-FrameworkInfo.plists in a path=>integrity map for easier readability of the snapshotcreateTestApp- Creates a new app at runtime from the app template, adds folders, symlinks, and allows you to pass inadditionalFilesthat you can use to customize each app's directory.binsnapshot files to triggerelectron/universalshimming logic flow