test: giving steroids to the test suite 💪

[mmaietta]

This is purely a test suite upgrade except for code change: asar.listPackage was missing a 2nd argument. I checked with electron/asar and its default behavior is isPack: 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.todo test cases.

Test Cases added:

• should not inject ElectronAsarIntegrity into 'infoPlistsToIgnore'
• should shim two different app folders

Functionally, this PR adds:

• verifyApp helper function does all the below:
  • executes ensureUniversal (consolidating existing logic)
  • verifyHeader - parses out unstable properties ("offset" varies per build machine?) and stores a snapshot
  • verifyFileTree - Traverses for app dirs (both merged and shimmed) and snapshots the tree hierarchy and plain text files (also used for unpacked dir snapshots)
  • extractAsarIntegrity - a loop verifies the ElectronAsarIntegrity entry in the Info.plist in all non-Framework Info.plists in a path=>integrity map for easier readability of the snapshot
• createTestApp - Creates a new app at runtime from the app template, adds folders, symlinks, and allows you to pass in additionalFiles that you can use to customize each app's directory
  • This was used for the test to mock two .bin snapshot files to trigger electron/universal shimming logic flow
  • It will be used for upcoming unit tests for universal app merging w/ unpacked asar assets

[dsanders11]

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.

[mmaietta]

Thank you for the feedback! Updated the PR to use the exposed public method

[dsanders11]

I believe the changes in this file can also be dropped now that the typings directory has been removed from the PR.

[mmaietta]

Oh derp, my apologies. Yep! I will push the change.

[BlackHole1]

Make the logic here clearer.

Suggested change

	!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))

[mmaietta]

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

[BlackHole1]

Oh. Good catch!

path.basename(p) === 'app' || path.basename(p).endsWith('.asar.unpacked')

[mmaietta]

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();

[BlackHole1]

Is this a spelling mistake? Maybe it should be framework or Frameworks.

[mmaietta]

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

[BlackHole1]

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?

[mmaietta]

Love it!

[BlackHole1]

Code LGTM. It's just a few issues with the code formatting. But it shouldn't be a reason to block the PR merge.