Skip to content
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

🐛 fix: Align cache hashes #1248

Open
wants to merge 13 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions packages/cache/src/cli/cliEntrypoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@ export const cliEntrypoint = async () => {
.addHelpText('afterAll', `\n${footer}\n`)
.parse(process.argv)

let walletSetupDir = program.args[0]
if (!walletSetupDir) {
walletSetupDir = path.join(process.cwd(), 'test', WALLET_SETUP_DIR_NAME)
let walletSetupDir = path.join(process.cwd(), 'test', WALLET_SETUP_DIR_NAME)

if (program.args[0]) {
walletSetupDir = path.join(process.cwd(), program.args[0])
}

const flags: CliFlags = program.opts()
Expand All @@ -47,7 +48,14 @@ export const cliEntrypoint = async () => {

if (flags.debug) {
console.log('[DEBUG] Running with the following options:')
console.log({ cacheDir: walletSetupDir, ...flags, headless: Boolean(process.env.HEADLESS) ?? false }, '\n')
console.log(
{
cacheDir: walletSetupDir,
...flags,
headless: Boolean(process.env.HEADLESS) ?? false
},
'\n'
)
}

if (os.platform() === 'win32') {
Expand All @@ -62,10 +70,15 @@ export const cliEntrypoint = async () => {
process.exit(1)
}

const compiledWalletSetupDirPath = await compileWalletSetupFunctions(walletSetupDir, flags.debug)
console.log(chalk.greenBright('🚀 Building the cache for wallet setup functions... 🚀\n'))

const { outDir: compiledWalletSetupDirPath, setupFunctionHashes } = await compileWalletSetupFunctions(
walletSetupDir,
flags.debug
)

// TODO: We should be using `prepareExtension` function from the wallet itself!
await createCache(compiledWalletSetupDirPath, prepareExtension, flags.force)
await createCache(compiledWalletSetupDirPath, setupFunctionHashes, prepareExtension, flags.force)

if (!flags.debug) {
await rimraf(compiledWalletSetupDirPath)
Expand Down
24 changes: 21 additions & 3 deletions packages/cache/src/cli/compileWalletSetupFunctions.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,25 @@
import path from 'node:path'
import fs from 'fs-extra'
import { glob } from 'glob'
import { build } from 'tsup'

import { ensureCacheDirExists } from '../ensureCacheDirExists'
import buildWalletSetupFunction from '../utils/buildWalletSetupFunction'
import { extractWalletSetupFunction } from '../utils/extractWalletSetupFunction'
import { getWalletSetupFuncHash } from '../utils/getWalletSetupFuncHash'
import { FIXES_BANNER } from './compilationFixes'

const OUT_DIR_NAME = 'wallet-setup-dist'
const OUT_DIR_NAME = '.wallet-setup-dist'

const createGlobPattern = (walletSetupDir: string) => path.join(walletSetupDir, '**', '*.setup.{ts,js,mjs}')

export async function compileWalletSetupFunctions(walletSetupDir: string, debug: boolean) {
const outDir = path.join(ensureCacheDirExists(), OUT_DIR_NAME)

fs.ensureDirSync(outDir)

const globPattern = createGlobPattern(walletSetupDir)
const fileList = await glob(globPattern)
const fileList = (await glob(globPattern)).sort()

if (debug) {
console.log('[DEBUG] Found the following wallet setup files:')
Expand Down Expand Up @@ -51,5 +58,16 @@ export async function compileWalletSetupFunctions(walletSetupDir: string, debug:
}
})

return outDir
const setupFunctionHashes = await Promise.all(
fileList.map(async (filePath) => {
const sourceCode = fs.readFileSync(filePath, 'utf8')
const functionString = extractWalletSetupFunction(sourceCode)

const rawFunctionBuild = buildWalletSetupFunction(functionString)

return getWalletSetupFuncHash(rawFunctionBuild)
})
)

return { outDir, setupFunctionHashes }
}
14 changes: 8 additions & 6 deletions packages/cache/src/createCache.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
import { getUniqueWalletSetupFunctions } from './utils/getUniqueWalletSetupFunctions'
import { triggerCacheCreation } from './utils/triggerCacheCreation'

export async function createCache(walletSetupDirPath: string, downloadExtension: () => Promise<string>, force = false) {
export async function createCache(
walletSetupDirPath: string,
hashes: string[],
downloadExtension: () => Promise<string>,
force = false
) {
const setupFunctions = await getUniqueWalletSetupFunctions(walletSetupDirPath)

const cacheCreationPromises = await triggerCacheCreation(setupFunctions, downloadExtension, force)
const cacheCreationOutput = await triggerCacheCreation(setupFunctions, hashes, downloadExtension, force)

if (cacheCreationPromises.length === 0) {
if (cacheCreationOutput.length === 0) {
console.log('No new setup functions to cache. Exiting...')
return
}

// TODO: This line has no unit test. Not sure how to do it. Look into it later.
await Promise.all(cacheCreationPromises)

console.log('All wallet setup functions are now cached!')
}
5 changes: 4 additions & 1 deletion packages/cache/src/defineWalletSetup.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { BrowserContext, Page } from 'playwright-core'
import buildWalletSetupFunction from './utils/buildWalletSetupFunction'
import { getWalletSetupFuncHash } from './utils/getWalletSetupFuncHash'

// TODO: Should we export this type in the `release` package?
Expand All @@ -15,7 +16,9 @@ export type WalletSetupFunction = (context: BrowserContext, walletPage: Page) =>
* @returns An object containing the hash of the function, the function itself, and the wallet password. The `testWithWalletSetup` function uses this object.
*/
export function defineWalletSetup(walletPassword: string, fn: WalletSetupFunction) {
const hash = getWalletSetupFuncHash(fn)
const walletSetupFunction = buildWalletSetupFunction(fn.toString())

const hash = getWalletSetupFuncHash(walletSetupFunction)

return {
hash,
Expand Down
15 changes: 15 additions & 0 deletions packages/cache/src/utils/buildWalletSetupFunction.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { transformSync } from 'esbuild'

export default function buildWalletSetupFunction(walletSetupFunctionString: string) {
const { code } = transformSync(walletSetupFunctionString, {
format: 'esm',
minifyWhitespace: true,
target: 'es2022',
drop: ['console', 'debugger'],
loader: 'ts',
logLevel: 'silent',
platform: 'node'
})

return code
}
12 changes: 12 additions & 0 deletions packages/cache/src/utils/extractWalletSetupFunction.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export function extractWalletSetupFunction(sourceCode: string): string {
const match = sourceCode.match(
/defineWalletSetup\s*\([^,]*,\s*(async\s*\([^)]*\)\s*=>\s*{(?:[^{}]*|{(?:[^{}]*|{[^{}]*})*})*})\s*\)/

Check failure

Code scanning / CodeQL

Inefficient regular expression High

This part of the regular expression may cause exponential backtracking on strings starting with 'defineWalletSetup(,async()=>{' and containing many repetitions of 'z'.

Copilot Autofix AI 17 days ago

To fix the problem, we need to modify the regular expression to remove the ambiguity and reduce the potential for excessive backtracking. This can be achieved by making the sub-expressions more specific and avoiding patterns that can match the same string in multiple ways. Specifically, we can replace [^{}]* with a more precise pattern that matches the intended input without allowing for excessive backtracking.

The best way to fix the problem is to rewrite the regular expression to avoid ambiguous sub-expressions and nested quantifiers. We can achieve this by using non-capturing groups and more specific character classes.

Suggested changeset 1
packages/cache/src/utils/extractWalletSetupFunction.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/cache/src/utils/extractWalletSetupFunction.ts b/packages/cache/src/utils/extractWalletSetupFunction.ts
--- a/packages/cache/src/utils/extractWalletSetupFunction.ts
+++ b/packages/cache/src/utils/extractWalletSetupFunction.ts
@@ -2,3 +2,3 @@
   const match = sourceCode.match(
-    /defineWalletSetup\s*\([^,]*,\s*(async\s*\([^)]*\)\s*=>\s*{(?:[^{}]*|{(?:[^{}]*|{[^{}]*})*})*})\s*\)/
+    /defineWalletSetup\s*\([^,]*,\s*(async\s*\([^)]*\)\s*=>\s*{(?:[^{}{}]*|{(?:[^{}{}]*|{[^{}{}]*})*})*})\s*\)/
   )
EOF
@@ -2,3 +2,3 @@
const match = sourceCode.match(
/defineWalletSetup\s*\([^,]*,\s*(async\s*\([^)]*\)\s*=>\s*{(?:[^{}]*|{(?:[^{}]*|{[^{}]*})*})*})\s*\)/
/defineWalletSetup\s*\([^,]*,\s*(async\s*\([^)]*\)\s*=>\s*{(?:[^{}{}]*|{(?:[^{}{}]*|{[^{}{}]*})*})*})\s*\)/
)
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options

Check failure

Code scanning / CodeQL

Inefficient regular expression High

This part of the regular expression may cause exponential backtracking on strings starting with 'defineWalletSetup(,async()=>{{' and containing many repetitions of 'z'.

Copilot Autofix AI 17 days ago

To fix the problem, we need to modify the regular expression to remove the ambiguity that leads to catastrophic backtracking. This can be achieved by making the pattern more specific and less ambiguous. In this case, we can replace the ambiguous [^{}]* with a more precise pattern that matches the expected structure of the input.

The best way to fix the problem without changing existing functionality is to rewrite the regular expression to avoid nested quantifiers and ambiguous patterns. Specifically, we can use a non-recursive approach to match the nested braces and their contents.

Suggested changeset 1
packages/cache/src/utils/extractWalletSetupFunction.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/cache/src/utils/extractWalletSetupFunction.ts b/packages/cache/src/utils/extractWalletSetupFunction.ts
--- a/packages/cache/src/utils/extractWalletSetupFunction.ts
+++ b/packages/cache/src/utils/extractWalletSetupFunction.ts
@@ -2,3 +2,3 @@
   const match = sourceCode.match(
-    /defineWalletSetup\s*\([^,]*,\s*(async\s*\([^)]*\)\s*=>\s*{(?:[^{}]*|{(?:[^{}]*|{[^{}]*})*})*})\s*\)/
+    /defineWalletSetup\s*\([^,]*,\s*(async\s*\([^)]*\)\s*=>\s*{[^{}]*(?:{[^{}]*}[^{}]*)*})\s*\)/
   )
EOF
@@ -2,3 +2,3 @@
const match = sourceCode.match(
/defineWalletSetup\s*\([^,]*,\s*(async\s*\([^)]*\)\s*=>\s*{(?:[^{}]*|{(?:[^{}]*|{[^{}]*})*})*})\s*\)/
/defineWalletSetup\s*\([^,]*,\s*(async\s*\([^)]*\)\s*=>\s*{[^{}]*(?:{[^{}]*}[^{}]*)*})\s*\)/
)
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
)

if (!match || !match[1]) {
console.log('Failed to extract defineWalletSetup callback from:', sourceCode)
throw new Error('Could not find defineWalletSetup callback')
}

return match[1]
}
2 changes: 1 addition & 1 deletion packages/cache/src/utils/getWalletSetupFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,5 @@ export async function getWalletSetupFiles(walletSetupDirPath: string) {
)
}

return fileList
return fileList.sort()
}
18 changes: 2 additions & 16 deletions packages/cache/src/utils/getWalletSetupFuncHash.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,12 @@
import { createHash } from 'node:crypto'
import esbuild from 'esbuild'

// Same length as the file part (first part before the `-`) of a Playwright Test ID.
export const WALLET_SETUP_FUNC_HASH_LENGTH = 10

// biome-ignore lint/suspicious/noExplicitAny: any type here is intentional
type AnyFunction = (...args: any) => Promise<any>

export function getWalletSetupFuncHash(walletSetupFunc: AnyFunction) {
// This transformation is necessary because a user could end up using a different execution engine than Playwright.
// Different execution engines -> different codes -> different hashes.
const { code } = esbuild.transformSync(walletSetupFunc.toString(), {
format: 'esm',
minifyWhitespace: true,
drop: ['console', 'debugger'],
loader: 'ts',
logLevel: 'silent'
})

export function getWalletSetupFuncHash(walletSetupString: string) {
const hash = createHash('shake256', {
outputLength: WALLET_SETUP_FUNC_HASH_LENGTH
})

return hash.update(code).digest('hex')
return hash.update(walletSetupString).digest('hex')
}
64 changes: 31 additions & 33 deletions packages/cache/src/utils/triggerCacheCreation.ts
Original file line number Diff line number Diff line change
@@ -1,52 +1,50 @@
import path from 'node:path'
import fs from 'fs-extra'
import type { WalletSetupFunction } from '../defineWalletSetup'
import { ensureCacheDirExists } from '../ensureCacheDirExists'
import { createCacheForWalletSetupFunction } from './createCacheForWalletSetupFunction'
import { getUniqueWalletSetupFunctions } from './getUniqueWalletSetupFunctions'
import { isDirEmpty } from './isDirEmpty'

export async function triggerCacheCreation(
setupFunctions: Awaited<ReturnType<typeof getUniqueWalletSetupFunctions>>,
setupFunctions: Map<string, { fileName: string; setupFunction: WalletSetupFunction }>,
hashes: string[],
downloadExtension: () => Promise<string>,
force: boolean
) {
const cacheDirPath = ensureCacheDirExists()
const extensionPath = await downloadExtension()

const cacheCreationPromises = []

for (const [funcHash, { fileName, setupFunction }] of setupFunctions) {
const cachePath = path.join(cacheDirPath, funcHash)
const doesCacheDirExist = await fs.exists(cachePath)
const isCacheDirEmpty = await isDirEmpty(cachePath)
return await Promise.all(
Array.from(setupFunctions).map(async ([_, { fileName, setupFunction }], index) => {
if (!hashes[index]) {
throw new Error(`No hash found for ${fileName}`)
}

if (doesCacheDirExist) {
if (isCacheDirEmpty) {
// In case of incorrect Playwright setup, the cache dir will be empty. For now, we're just deleting it.
await fs.remove(cachePath)
} else {
if (!force) {
console.log(`Cache already exists for ${funcHash}. Skipping...`)
continue
}
const funcHash = hashes[index]

console.log(`Cache already exists for ${funcHash} but force flag is set. Deleting cache...`)
await fs.remove(cachePath)
}
}
const cachePath = path.join(cacheDirPath, funcHash || 'unknown')
const doesCacheDirExist = await fs.exists(cachePath)
const isCacheDirEmpty = await isDirEmpty(cachePath)

const fileNameWithCorrectExtension = fileName.replace(/\.(ts|js|mjs)$/, '.{ts,js,mjs}')
console.log(`Triggering cache creation for: ${funcHash} (${fileNameWithCorrectExtension})`)
if (doesCacheDirExist) {
if (isCacheDirEmpty) {
// In case of incorrect Playwright setup, the cache dir will be empty. For now, we're just deleting it.
await fs.remove(cachePath)
} else {
if (!force) {
console.log(`Cache already exists for ${funcHash}. Skipping...`)
return
}

// We're not inferring the return type here to make sure we don't accidentally await the function.
const createCachePromise: Promise<void> = createCacheForWalletSetupFunction(
extensionPath,
cachePath,
setupFunction,
fileNameWithCorrectExtension
)
cacheCreationPromises.push(createCachePromise)
}
console.log(`Cache already exists for ${funcHash} but force flag is set. Deleting cache...`)
await fs.remove(cachePath)
}
}

return cacheCreationPromises
const fileNameWithCorrectExtension = fileName.replace(/\.(ts|js|mjs)$/, '.{ts,js,mjs}')
console.log(`Triggering cache creation for: ${funcHash} (${fileNameWithCorrectExtension})`)
// We're not inferring the return type here to make sure we don't accidentally await the function.
return createCacheForWalletSetupFunction(extensionPath, cachePath, setupFunction, fileNameWithCorrectExtension)
})
)
}
27 changes: 19 additions & 8 deletions packages/cache/test/createCache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,20 @@ const ROOT_DIR = '/tmp'

const setupFunctions = new Map<string, { fileName: string; setupFunction: WalletSetupFunction }>()

setupFunctions.set('hash1', { fileName: path.join(ROOT_DIR, 'hash1'), setupFunction: vi.fn() })
setupFunctions.set('hash2', { fileName: path.join(ROOT_DIR, 'hash2'), setupFunction: vi.fn() })
setupFunctions.set('hash3', { fileName: path.join(ROOT_DIR, 'hash3'), setupFunction: vi.fn() })
setupFunctions.set('hash1', {
fileName: path.join(ROOT_DIR, 'hash1'),
setupFunction: vi.fn()
})
setupFunctions.set('hash2', {
fileName: path.join(ROOT_DIR, 'hash2'),
setupFunction: vi.fn()
})
setupFunctions.set('hash3', {
fileName: path.join(ROOT_DIR, 'hash3'),
setupFunction: vi.fn()
})

const functionStrings = ['function1', 'function2', 'function3']

vi.mock('../src/utils/getUniqueWalletSetupFunctions', async () => {
return {
Expand Down Expand Up @@ -44,7 +55,7 @@ describe('createCache', () => {
it('calls getUniqueWalletSetupFunctions with correct arguments', async () => {
const getUniqueWalletSetupFunctionsSpy = vi.spyOn(GetUniqueWalletSetupFunctions, 'getUniqueWalletSetupFunctions')

await createCache(ROOT_DIR, vi.fn(), false)
await createCache(ROOT_DIR, functionStrings, vi.fn(), false)

expect(getUniqueWalletSetupFunctionsSpy).toHaveBeenCalledOnce()
expect(getUniqueWalletSetupFunctionsSpy).toHaveBeenCalledWith(ROOT_DIR)
Expand All @@ -54,23 +65,23 @@ describe('createCache', () => {
const triggerCacheCreationSpy = vi.spyOn(TriggerCacheCreation, 'triggerCacheCreation')

const downloadExtension = vi.fn(async () => path.join(ROOT_DIR, 'extension'))
await createCache(ROOT_DIR, downloadExtension, false)
await createCache(ROOT_DIR, functionStrings, downloadExtension, false)

expect(triggerCacheCreationSpy).toHaveBeenCalledOnce()
expect(triggerCacheCreationSpy).toHaveBeenCalledWith(setupFunctions, downloadExtension, false)
expect(triggerCacheCreationSpy).toHaveBeenCalledWith(setupFunctions, functionStrings, downloadExtension, false)
})

it('does nothing if no setup functions need caching', async () => {
vi.spyOn(TriggerCacheCreation, 'triggerCacheCreation').mockResolvedValueOnce([])

await createCache(ROOT_DIR, vi.fn(), false)
await createCache(ROOT_DIR, functionStrings, vi.fn(), false)

expect(consoleLogSpy).toHaveBeenCalledOnce()
expect(consoleLogSpy).toHaveBeenCalledWith('No new setup functions to cache. Exiting...')
})

it('console.logs at the end', async () => {
await createCache(ROOT_DIR, vi.fn(), false)
await createCache(ROOT_DIR, functionStrings, vi.fn(), false)

expect(consoleLogSpy).toHaveBeenCalledOnce()
expect(consoleLogSpy).toHaveBeenCalledWith('All wallet setup functions are now cached!')
Expand Down
2 changes: 1 addition & 1 deletion packages/cache/test/defineWalletSetup.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, expect, it } from 'vitest'
import { defineWalletSetup } from '../src/defineWalletSetup'
import { defineWalletSetup } from '../src'

const PASSWORD = 'Quack Quack! 🦆'
const EXPECTED_HASH = 'f9c5ea5bb2c3aac96ff4'
Expand Down
14 changes: 6 additions & 8 deletions packages/cache/test/utils/getWalletSetupFuncHash.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
import { describe, expect, it } from 'vitest'
import { WALLET_SETUP_FUNC_HASH_LENGTH, getWalletSetupFuncHash } from '../../src/utils/getWalletSetupFuncHash'

const EXPECTED_HASH = 'b940c886be3c1a041ddd'

const testFunction = async (name: string) => {
return `Hello ${name}!`
}
const EXPECTED_HASH = '117dc0b7e0dd758cfee3'

describe('getWalletSetupFuncHash', () => {
it('throws an error if esbuild transformation fails', async () => {
Expand All @@ -14,16 +10,18 @@ describe('getWalletSetupFuncHash', () => {
// biome-ignore lint/suspicious/noExplicitAny: any type here is intentional
} as any

expect(() => getWalletSetupFuncHash(incorrectFunctionObject)).toThrowError('Transform failed with 1 error')
expect(() => getWalletSetupFuncHash(incorrectFunctionObject)).toThrowError(
'The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received an instance of Object'
)
})

it('returns hash', async () => {
const hash = getWalletSetupFuncHash(testFunction)
const hash = getWalletSetupFuncHash('test_test')
drptbl marked this conversation as resolved.
Show resolved Hide resolved
expect(hash).toEqual(EXPECTED_HASH)
})

it('returns hash of a correct length', async () => {
const hash = getWalletSetupFuncHash(testFunction)
const hash = getWalletSetupFuncHash('test_test2')

// We multiply by 2 because the hash is in a hex format, i.e. each byte is represented by 2 characters.
expect(hash.length).toEqual(2 * WALLET_SETUP_FUNC_HASH_LENGTH)
Expand Down
Loading
Loading