Skip to content

Fix/tmp dir build #16859

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Fix/tmp dir build #16859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6efac13
Fix typo in test log message in e2e-tests script
Jul 11, 2025
51277ce
Remove redundant command and enhance file copy with ignore logic
Jul 11, 2025
5dc6dce
Filter files in `copyToTemp` using .gitignore rules
Jul 11, 2025
af78dda
Refactor asyncExec to improve process output handling
Jul 11, 2025
0420ec5
Refactor e2e test cleanup and temporary directory handling
Jul 11, 2025
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
7 changes: 7 additions & 0 deletions dev-packages/e2e-tests/clean.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { tmpdir } from 'os';
import { join } from 'path';
import { rimrafSync } from 'rimraf';

const buildDir = join(tmpdir(), 'sentry-e2e-tests-*');

process.exit(Number(!rimrafSync([...process.argv.slice(2), buildDir], { glob: true })));
27 changes: 24 additions & 3 deletions dev-packages/e2e-tests/lib/copyToTemp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,32 @@
/* eslint-disable no-console */
import { readFileSync, writeFileSync } from 'fs';
import { existsSync, readFileSync, writeFileSync } from 'fs';
import { cp } from 'fs/promises';
import { join } from 'path';
import ignore from 'ignore';
import { dirname, join, relative } from 'path';

export async function copyToTemp(originalPath: string, tmpDirPath: string): Promise<void> {
// copy files to tmp dir
await cp(originalPath, tmpDirPath, { recursive: true });
const ig = ignore();
const ignoreFileDirs = [
originalPath,
dirname(__dirname)
]
ig.add(['.gitignore', 'node_modules', 'dist', 'build']);
Copy link
Member

Choose a reason for hiding this comment

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

is it safe to not copy node_modules? I'm not familiar yet with the temp dir change (which is why I requested a review from @mydea) so it might be fine, if we install deps after we copied everything to the temp dir.

Copy link
Member

Choose a reason for hiding this comment

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

this should be fine, we install dependencies inside of the tmp dir then. However, this should not really be necessary after this has settled - if you just run yarn clean:all once, all of these things will not exist in the monorepo anymore anyhow 🤔 this is why I did not add any handling for this, not sure if we want to add this complexity here...?

TLDR this is only a "problem" if you have stale state from previous local runs. Running yarn clean should actually get rid of this fully.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you are right, but... Usually , while developing tests, you will run them more than once (locally) and after each run you have to do pnpm install inside test-app dir. That's why this PR, also, removes:
await asyncExec('pnpm clean:test-applications', { env, cwd: __dirname });
It is useless and annoying because test-app is being copied to tmp dir.

for(const dir of ignoreFileDirs) {
const ignore_file = join(dir, '.gitignore');
if (existsSync(ignore_file)) {
ig.add(readFileSync(ignore_file, 'utf8'));
}
}

await cp(originalPath, tmpDirPath, {
recursive: true,
filter: src => {
const relPath = relative(originalPath, src);
if (!relPath) return true;
return !ig.ignores(relPath);
},
});

fixPackageJson(tmpDirPath);
}
Expand Down
2 changes: 1 addition & 1 deletion dev-packages/e2e-tests/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"test:validate-test-app-setups": "ts-node validate-test-app-setups.ts",
"test:prepare": "ts-node prepare.ts",
"test:validate": "run-s test:validate-configuration test:validate-test-app-setups",
"clean": "rimraf tmp node_modules && yarn clean:test-applications && yarn clean:pnpm",
"clean": "ts-node ./clean.ts node_modules && yarn clean:test-applications && yarn clean:pnpm",
"ci:build-matrix": "ts-node ./lib/getTestMatrix.ts",
"ci:build-matrix-optional": "ts-node ./lib/getTestMatrix.ts --optional=true",
"ci:copy-to-temp": "ts-node ./ciCopyToTemp.ts",
Expand Down
27 changes: 11 additions & 16 deletions dev-packages/e2e-tests/run.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,21 @@
/* eslint-disable no-console */
import { spawn } from 'child_process';
import { type SpawnOptions, spawn } from 'child_process';
import * as dotenv from 'dotenv';
import { mkdtemp, rm } from 'fs/promises';
import { sync as globSync } from 'glob';
import { tmpdir } from 'os';
import { join, resolve } from 'path';
import { basename, join, resolve } from 'path';
import { rimraf } from 'rimraf';
import { copyToTemp } from './lib/copyToTemp';
import { registrySetup } from './registrySetup';

const DEFAULT_DSN = 'https://username@domain/123';
const DEFAULT_SENTRY_ORG_SLUG = 'sentry-javascript-sdks';
const DEFAULT_SENTRY_PROJECT = 'sentry-javascript-e2e-tests';

function asyncExec(command: string, options: { env: Record<string, string | undefined>; cwd: string }): Promise<void> {
function asyncExec(command: string, options: Omit<SpawnOptions, 'shell'|'stdio'>): Promise<void> {
return new Promise((resolve, reject) => {
const process = spawn(command, { ...options, shell: true });

process.stdout.on('data', data => {
console.log(`${data}`);
});

process.stderr.on('data', data => {
console.error(`${data}`);
});
const process = spawn(command, { ...options, shell: true, stdio: ['ignore', 'inherit', 'inherit'] });
Comment on lines +16 to +18
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not necessary, but why not to have it more clean


process.on('error', error => {
reject(error);
Expand Down Expand Up @@ -64,21 +57,23 @@ async function run(): Promise<void> {
console.log('Cleaning test-applications...');
console.log('');

const tmpPrefix = join(tmpdir(), 'sentry-e2e-tests-')
await rimraf(`${tmpPrefix}*`, { glob: true });
Comment on lines +60 to +61
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is here, because yarn clean will also delete everything inside test-apps, which is good, but not with active developing. And by doing like this, I believe that we will be able to debut issues, between the executions


if (!process.env.SKIP_REGISTRY) {
registrySetup();
}

await asyncExec('pnpm clean:test-applications', { env, cwd: __dirname });
await asyncExec('pnpm cache delete "@sentry/*"', { env, cwd: __dirname });

const testAppPaths = appName ? [appName.trim()] : globSync('*', { cwd: `${__dirname}/test-applications/` });

console.log(`Runnings tests for: ${testAppPaths.join(', ')}`);
console.log(`Running tests for: ${testAppPaths.join(', ')}`);
console.log('');

for (const testAppPath of testAppPaths) {
const originalPath = resolve('test-applications', testAppPath);
const tmpDirPath = await mkdtemp(join(tmpdir(), `sentry-e2e-tests-${appName}-`));
const originalPath = resolve(__dirname, 'test-applications', testAppPath);
const tmpDirPath = await mkdtemp(`${tmpPrefix}${basename(testAppPath)}-`);

await copyToTemp(originalPath, tmpDirPath);
const cwd = tmpDirPath;
Expand Down
Loading