From 7f6ce805ef1f51b6911a0910d28fe83a5638d237 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 28 Aug 2024 13:17:43 +0200 Subject: [PATCH 1/2] test: Avoid race conditions with symlinks Also, no need to double-symlink stuff, and since we now build into unique dirs, we can skip the check for existing symlinks as well. --- .../utils/generatePlugin.ts | 32 +++++++++---------- .../utils/staticAssets.ts | 19 +---------- 2 files changed, 16 insertions(+), 35 deletions(-) diff --git a/dev-packages/browser-integration-tests/utils/generatePlugin.ts b/dev-packages/browser-integration-tests/utils/generatePlugin.ts index 30939c40c955..9189ce63812f 100644 --- a/dev-packages/browser-integration-tests/utils/generatePlugin.ts +++ b/dev-packages/browser-integration-tests/utils/generatePlugin.ts @@ -4,7 +4,7 @@ import type { Package } from '@sentry/types'; import HtmlWebpackPlugin, { createHtmlTagObject } from 'html-webpack-plugin'; import type { Compiler } from 'webpack'; -import { addStaticAsset, addStaticAssetSymlink } from './staticAssets'; +import { addStaticAsset, symlinkAsset } from './staticAssets'; const LOADER_TEMPLATE = fs.readFileSync(path.join(__dirname, '../fixtures/loader.js'), 'utf-8'); const PACKAGES_DIR = path.join(__dirname, '..', '..', '..', 'packages'); @@ -214,7 +214,10 @@ class SentryScenarioGenerationPlugin { src: 'cdn.bundle.js', }); - addStaticAssetSymlink(this.localOutPath, path.resolve(PACKAGES_DIR, bundleName, bundlePath), 'cdn.bundle.js'); + symlinkAsset( + path.resolve(PACKAGES_DIR, bundleName, bundlePath), + path.join(this.localOutPath, 'cdn.bundle.js'), + ); if (useLoader) { const loaderConfig = LOADER_CONFIGS[bundleKey]; @@ -245,14 +248,13 @@ class SentryScenarioGenerationPlugin { const fileName = `${integration}.bundle.js`; // We add the files, but not a script tag - they are lazy-loaded - addStaticAssetSymlink( - this.localOutPath, + symlinkAsset( path.resolve( PACKAGES_DIR, 'feedback', BUNDLE_PATHS['feedback']?.[integrationBundleKey]?.replace('[INTEGRATION_NAME]', integration) || '', ), - fileName, + path.join(this.localOutPath, fileName), ); }); } @@ -262,26 +264,23 @@ class SentryScenarioGenerationPlugin { if (baseIntegrationFileName) { this.requiredIntegrations.forEach(integration => { const fileName = `${integration}.bundle.js`; - addStaticAssetSymlink( - this.localOutPath, + symlinkAsset( path.resolve( PACKAGES_DIR, 'browser', baseIntegrationFileName.replace('[INTEGRATION_NAME]', integration), ), - fileName, + path.join(this.localOutPath, fileName), ); if (integration === 'feedback') { - addStaticAssetSymlink( - this.localOutPath, + symlinkAsset( path.resolve(PACKAGES_DIR, 'feedback', 'build/bundles/feedback-modal.js'), - 'feedback-modal.bundle.js', + path.join(this.localOutPath, 'feedback-modal.bundle.js'), ); - addStaticAssetSymlink( - this.localOutPath, + symlinkAsset( path.resolve(PACKAGES_DIR, 'feedback', 'build/bundles/feedback-screenshot.js'), - 'feedback-screenshot.bundle.js', + path.join(this.localOutPath, 'feedback-screenshot.bundle.js'), ); } @@ -295,10 +294,9 @@ class SentryScenarioGenerationPlugin { const baseWasmFileName = BUNDLE_PATHS['wasm']?.[integrationBundleKey]; if (this.requiresWASMIntegration && baseWasmFileName) { - addStaticAssetSymlink( - this.localOutPath, + symlinkAsset( path.resolve(PACKAGES_DIR, 'wasm', baseWasmFileName), - 'wasm.bundle.js', + path.join(this.localOutPath, 'wasm.bundle.js'), ); const wasmObject = createHtmlTagObject('script', { diff --git a/dev-packages/browser-integration-tests/utils/staticAssets.ts b/dev-packages/browser-integration-tests/utils/staticAssets.ts index 447a3ad337f7..8cfd25c8fdae 100644 --- a/dev-packages/browser-integration-tests/utils/staticAssets.ts +++ b/dev-packages/browser-integration-tests/utils/staticAssets.ts @@ -22,23 +22,6 @@ export function addStaticAsset(localOutPath: string, fileName: string, cb: () => symlinkAsset(newPath, path.join(localOutPath, fileName)); } -export function addStaticAssetSymlink(localOutPath: string, originalPath: string, fileName: string): void { - const newPath = path.join(STATIC_DIR, fileName); - - // Only copy files once - if (!fs.existsSync(newPath)) { - fs.symlinkSync(originalPath, newPath); - } - - symlinkAsset(newPath, path.join(localOutPath, fileName)); -} - -function symlinkAsset(originalPath: string, targetPath: string): void { - try { - fs.unlinkSync(targetPath); - } catch { - // ignore errors here - } - +export function symlinkAsset(originalPath: string, targetPath: string): void { fs.linkSync(originalPath, targetPath); } From 44d1d00766fc8c9d4b7672b384b4eeb9f338d1ef Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 28 Aug 2024 13:44:10 +0200 Subject: [PATCH 2/2] fix test --- .../browser-integration-tests/utils/staticAssets.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/dev-packages/browser-integration-tests/utils/staticAssets.ts b/dev-packages/browser-integration-tests/utils/staticAssets.ts index 8cfd25c8fdae..81c18eec1dcf 100644 --- a/dev-packages/browser-integration-tests/utils/staticAssets.ts +++ b/dev-packages/browser-integration-tests/utils/staticAssets.ts @@ -23,5 +23,10 @@ export function addStaticAsset(localOutPath: string, fileName: string, cb: () => } export function symlinkAsset(originalPath: string, targetPath: string): void { - fs.linkSync(originalPath, targetPath); + try { + fs.linkSync(originalPath, targetPath); + } catch { + // ignore errors here, probably means the file already exists + // Since we always build into a new directory for each test, we can safely ignore this + } }