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 3 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
1 change: 1 addition & 0 deletions examples/metamask/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"dependencies": {
"@playwright/test": "1.48.2",
"@synthetixio/synpress": "workspace:*",
"@synthetixio/synpress-cache": "workspace:*",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you shouldn't need anything else other than synpress package in your tests

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

. + wallet package

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use dependency of dependency, and both of them are internal packages, turbo doesn't refresh the dependencies properly.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, but if I understand correctly, end user behavior will change and he will need two separate packages now, one for cache. it's breaking change which I would like to avoid.

Copy link
Collaborator

@drptbl drptbl Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, again - I think that end user should need main synpress package and nothing else. only additional wallets. we don't want to overcomplicate setup.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no package behaviour change, the only problem is that we have examples in monorepo, not outside but I can remove this dependency now

"dotenv": "16.4.2"
},
"devDependencies": {
Expand Down
5 changes: 5 additions & 0 deletions examples/metamask/test/config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default {
baseUrl: 'https://www.alphabot.app/',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is alphabot

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the page user added as an example - this test case is more for you as a prove that this issue has been fixed

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seedPhrase: 'test test test test test test test test test test test junk',
metamaskPassword: 'SynpressIsAwesomeNow!!!'
}
10 changes: 10 additions & 0 deletions examples/metamask/test/playwright/04_externalConfig.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { testWithSynpress } from '@synthetixio/synpress'
import { metaMaskFixtures } from '@synthetixio/synpress/playwright'
import config from '../config'
import importedConfigSetup from '../wallet-setup/importedConfig.setup'

const test = testWithSynpress(metaMaskFixtures(importedConfigSetup))

test('Properly generate cache', async ({ page }) => {
await page.goto(config.baseUrl)
})
27 changes: 27 additions & 0 deletions examples/metamask/test/wallet-setup/importedConfig.setup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { defineWalletSetup } from '@synthetixio/synpress-cache'
import { MetaMask, getExtensionId } from '@synthetixio/synpress/playwright'
import 'dotenv/config'
import config from '../config'

const SEED_PHRASE = process.env.SEED_PHRASE
const PASSWORD = process.env.WALLET_PASSWORD

export default defineWalletSetup(PASSWORD, async (context, walletPage) => {
const extensionId = await getExtensionId(context, 'MetaMask')

const metamask = new MetaMask(context, walletPage, PASSWORD, extensionId)

await metamask.importWallet(SEED_PHRASE)

const page = await context.newPage()

await page.goto(config.baseUrl)
await page.getByRole('button', { name: 'Sign in' }).click()
await page.getByRole('button', { name: 'MetaMask' }).click()

await metamask.connectToDapp()

await page.waitForTimeout(2000)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we shouldnt need any waits, especially with timeout, they are flaky and bad practice

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's just the same user example

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool, but these are our official examples in our main repo - which are going to be copied over by other people. we don't want to have bad practices here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after review I will remove it, feel free to use it as a testing playground

await metamask.confirmSignature()
await page.waitForTimeout(2000)
})
6 changes: 3 additions & 3 deletions packages/cache/src/cli/cliEntrypoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ export const cliEntrypoint = async () => {
process.exit(1)
}

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

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

if (!flags.debug) {
await rimraf(compiledWalletSetupDirPath)
Expand Down
69 changes: 45 additions & 24 deletions packages/cache/src/cli/compileWalletSetupFunctions.ts
Original file line number Diff line number Diff line change
@@ -1,55 +1,76 @@
import path from 'node:path'
import { glob } from 'glob'
import { build } from 'tsup'
import { ensureCacheDirExists } from '../ensureCacheDirExists'
import { FIXES_BANNER } from './compilationFixes'
import path from "node:path";
import { glob } from "glob";
import { build } from "tsup";
import { ensureCacheDirExists } from "../ensureCacheDirExists";
import { FIXES_BANNER } from "./compilationFixes";
import buildWalletSetupFunction from "../utils/buildWalletSetupFunction";

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}')
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)
export async function compileWalletSetupFunctions(
walletSetupDir: string,
debug: boolean
) {
const outDir = path.join(ensureCacheDirExists(), OUT_DIR_NAME);

const globPattern = createGlobPattern(walletSetupDir)
const fileList = await glob(globPattern)
const globPattern = createGlobPattern(walletSetupDir);
const fileList = await glob(globPattern);

if (debug) {
console.log('[DEBUG] Found the following wallet setup files:')
console.log(fileList, '\n')
console.log("[DEBUG] Found the following wallet setup files:");
console.log(fileList, "\n");
}

// TODO: This error message is copied over from another function. Refactor this.
if (!fileList.length) {
throw new Error(
[
`No wallet setup files found at ${walletSetupDir}`,
'Remember that all wallet setup files must end with `.setup.{ts,js,mjs}` extension!'
].join('\n')
)
"Remember that all wallet setup files must end with `.setup.{ts,js,mjs}` extension!",
].join("\n")
);
}

await build({
name: 'cli-build',
name: "cli-build",
silent: true,
entry: fileList,
clean: true,
outDir,
format: 'esm',
format: "esm",
splitting: true,
sourcemap: false,
config: false,
// TODO: Make this list configurable.
external: ['@synthetixio/synpress', '@playwright/test', 'playwright-core', 'esbuild', 'tsup'],
external: [
"@synthetixio/synpress",
"@playwright/test",
"playwright-core",
"esbuild",
"tsup",
],
banner: {
js: FIXES_BANNER
js: FIXES_BANNER,
},
esbuildOptions(options) {
// TODO: In this step, if the debug file is present, we should modify `console.log` so it prints from which file the log is coming from.
// We're dropping `console.log` and `debugger` statements because they do not play nicely with the Playwright Test Runner.
options.drop = debug ? [] : ['console', 'debugger']
}
})
options.drop = debug ? [] : ["console", "debugger"];
},
});

const functionStrings = await Promise.all(
fileList.map(async (fileName) => {
const walletSetupFunction = await import(fileName);

return buildWalletSetupFunction(walletSetupFunction.toString());
})
);

console.log({functionStrings})
Fixed Show fixed Hide fixed

return outDir
return { outDir, functionStrings: functionStrings };
}
30 changes: 21 additions & 9 deletions packages/cache/src/createCache.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,30 @@
import { getUniqueWalletSetupFunctions } from './utils/getUniqueWalletSetupFunctions'
import { triggerCacheCreation } from './utils/triggerCacheCreation'
import { getUniqueWalletSetupFunctions } from "./utils/getUniqueWalletSetupFunctions";
import { triggerCacheCreation } from "./utils/triggerCacheCreation";

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

const cacheCreationPromises = await triggerCacheCreation(setupFunctions, downloadExtension, force)
const cacheCreationPromises = await triggerCacheCreation(
setupFunctions,
functionStrings,
downloadExtension,
force
);

if (cacheCreationPromises.length === 0) {
console.log('No new setup functions to cache. Exiting...')
return
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)
await Promise.all(cacheCreationPromises);

console.log('All wallet setup functions are now cached!')
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,5 +1,6 @@
import type { BrowserContext, Page } from 'playwright-core'
import { getWalletSetupFuncHash } from './utils/getWalletSetupFuncHash'
import buildWalletSetupFunction from './utils/buildWalletSetupFunction';

// TODO: Should we export this type in the `release` package?
export type WalletSetupFunction = (context: BrowserContext, walletPage: Page) => Promise<void>
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
19 changes: 19 additions & 0 deletions packages/cache/src/utils/buildWalletSetupFunction.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { transformSync } from "esbuild";
import { FIXES_BANNER } from "../cli/compilationFixes";

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",
banner: FIXES_BANNER,
});

return code;
}
29 changes: 8 additions & 21 deletions packages/cache/src/utils/getWalletSetupFuncHash.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,13 @@
import { createHash } from 'node:crypto'
import esbuild from 'esbuild'
import { createHash } from "node:crypto";

// Same length as the file part (first part before the `-`) of a Playwright Test ID.
export const WALLET_SETUP_FUNC_HASH_LENGTH = 10
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(walletSetupString: string) {
const hash = createHash("shake256", {
outputLength: WALLET_SETUP_FUNC_HASH_LENGTH,
});

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'
})

const hash = createHash('shake256', {
outputLength: WALLET_SETUP_FUNC_HASH_LENGTH
})

return hash.update(code).digest('hex')
return hash.update(walletSetupString).digest("hex");
}

97 changes: 54 additions & 43 deletions packages/cache/src/utils/triggerCacheCreation.ts
Original file line number Diff line number Diff line change
@@ -1,52 +1,63 @@
import path from 'node:path'
import fs from 'fs-extra'
import { ensureCacheDirExists } from '../ensureCacheDirExists'
import { createCacheForWalletSetupFunction } from './createCacheForWalletSetupFunction'
import { getUniqueWalletSetupFunctions } from './getUniqueWalletSetupFunctions'
import { isDirEmpty } from './isDirEmpty'
import path from "node:path";
import fs from "fs-extra";
import { ensureCacheDirExists } from "../ensureCacheDirExists";
import { createCacheForWalletSetupFunction } from "./createCacheForWalletSetupFunction";
import { isDirEmpty } from "./isDirEmpty";
import { getWalletSetupFuncHash } from "./getWalletSetupFuncHash";
import type { WalletSetupFunction } from "../defineWalletSetup";

export async function triggerCacheCreation(
setupFunctions: Awaited<ReturnType<typeof getUniqueWalletSetupFunctions>>,
setupFunctions: Map<
string,
{ fileName: string; setupFunction: WalletSetupFunction }
>,
functionStrings: 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)

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 cacheDirPath = ensureCacheDirExists();
const extensionPath = await downloadExtension();

return Array.from(setupFunctions).map(
async ([_, { fileName, setupFunction }], index) => {
// @ts-ignore
const funcHash = getWalletSetupFuncHash(functionStrings[index]);

const cachePath = path.join(cacheDirPath, funcHash);
const doesCacheDirExist = await fs.exists(cachePath);
const isCacheDirEmpty = await isDirEmpty(cachePath);

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;
}

console.log(
`Cache already exists for ${funcHash} but force flag is set. Deleting cache...`
);
await fs.remove(cachePath);
}

console.log(`Cache already exists for ${funcHash} but force flag is set. Deleting cache...`)
await fs.remove(cachePath)
}
}

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.
const createCachePromise: Promise<void> = createCacheForWalletSetupFunction(
extensionPath,
cachePath,
setupFunction,
fileNameWithCorrectExtension
)
cacheCreationPromises.push(createCachePromise)
}

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
);
}
);
}
Loading
Loading