Skip to content

fix(e2e): Fix various issues with concurrent E2E and Canary tests #7805

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

Merged
merged 17 commits into from
Apr 11, 2023
8 changes: 7 additions & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,7 @@ jobs:
yarn test:integration:ci

job_e2e_tests:
name: E2E Tests
name: E2E Tests (Shard ${{ matrix.shard }})
# We only run E2E tests for non-fork PRs because the E2E tests require secrets to work and they can't be accessed from forks
# Dependabot PRs sadly also don't have access to secrets, so we skip them as well
if:
Expand All @@ -758,6 +758,10 @@ jobs:
needs: [job_get_metadata, job_build]
runs-on: ubuntu-20.04
timeout-minutes: 20
strategy:
fail-fast: false
matrix:
shard: [1, 2, 3]
steps:
- name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }})
uses: actions/checkout@v3
Expand All @@ -782,6 +786,8 @@ jobs:
E2E_TEST_DSN: ${{ secrets.E2E_TEST_DSN }}
E2E_TEST_SENTRY_ORG_SLUG: 'sentry-javascript-sdks'
E2E_TEST_SENTRY_TEST_PROJECT: 'sentry-javascript-e2e-tests'
E2E_TEST_SHARD: ${{ matrix.shard }}
E2E_TEST_SHARD_AMOUNT: 3
run: |
cd packages/e2e-tests
yarn test:e2e
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/canary.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
job_canary_test:
name: Canary Tests
runs-on: ubuntu-20.04
timeout-minutes: 30
timeout-minutes: 60
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These tests are friggin slow for some reason

steps:
- name: 'Check out current commit'
uses: actions/checkout@v3
Expand Down
3 changes: 3 additions & 0 deletions packages/e2e-tests/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,6 @@ E2E_TEST_AUTH_TOKEN=
E2E_TEST_DSN=
E2E_TEST_SENTRY_ORG_SLUG=
E2E_TEST_SENTRY_TEST_PROJECT=
E2E_TEST_SHARD= # optional
E2E_TEST_SHARD_AMOUNT= # optional
CANARY_E2E_TEST= # optional
2 changes: 1 addition & 1 deletion packages/e2e-tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ To get you started with the recipe, you can copy the following into `test-recipe
{
"$schema": "../../test-recipe-schema.json",
"testApplicationName": "My New Test Application",
"buildCommand": "yarn install --network-concurrency 1",
"buildCommand": "yarn install",
"tests": [
{
"testName": "My new test",
Expand Down
24 changes: 18 additions & 6 deletions packages/e2e-tests/lib/buildApp.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
/* eslint-disable no-console */

import * as fs from 'fs-extra';
import * as os from 'os';
import * as path from 'path';

import { DEFAULT_BUILD_TIMEOUT_SECONDS } from './constants';
import type { Env, RecipeInstance } from './types';
import { spawnAsync } from './utils';
import { prefixObjectKeys, spawnAsync } from './utils';

export async function buildApp(appDir: string, recipeInstance: RecipeInstance, env: Env): Promise<void> {
export async function buildApp(appDir: string, recipeInstance: RecipeInstance, envVars: Env): Promise<void> {
const { recipe, label, dependencyOverrides } = recipeInstance;

const packageJsonPath = path.resolve(appDir, 'package.json');
Expand All @@ -28,13 +29,23 @@ export async function buildApp(appDir: string, recipeInstance: RecipeInstance, e
if (recipe.buildCommand) {
console.log(`Running build command for test application "${label}"`);

fs.mkdirSync(path.join(os.tmpdir(), 'e2e-test-yarn-caches'), { recursive: true });
const tempYarnCache = fs.mkdtempSync(path.join(os.tmpdir(), 'e2e-test-yarn-caches', 'cache-'));

const env = {
...process.env,
...envVars,
YARN_CACHE_FOLDER: tempYarnCache, // Use a separate yarn cache for each build commmand because multiple yarn commands running at the same time may corrupt the cache
};

const buildResult = await spawnAsync(recipe.buildCommand, {
cwd: appDir,
timeout: (recipe.buildTimeoutSeconds ?? DEFAULT_BUILD_TIMEOUT_SECONDS) * 1000,
env: {
...process.env,
...env,
} as unknown as NodeJS.ProcessEnv,
...prefixObjectKeys(env, 'NEXT_PUBLIC_'),
...prefixObjectKeys(env, 'REACT_APP_'),
},
});

if (buildResult.error) {
Expand All @@ -57,9 +68,10 @@ export async function buildApp(appDir: string, recipeInstance: RecipeInstance, e
cwd: appDir,
timeout: (recipe.buildTimeoutSeconds ?? DEFAULT_BUILD_TIMEOUT_SECONDS) * 1000,
env: {
...process.env,
...env,
} as unknown as NodeJS.ProcessEnv,
...prefixObjectKeys(env, 'NEXT_PUBLIC_'),
...prefixObjectKeys(env, 'REACT_APP_'),
},
},
buildResult.stdout,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@ import * as fs from 'fs';

import type { Recipe, RecipeInput, RecipeInstance } from './types';

export function buildRecipeInstances(recipePaths: string[]): RecipeInstance[] {
export function constructRecipeInstances(recipePaths: string[]): RecipeInstance[] {
const recipes = buildRecipes(recipePaths);
const recipeInstances: RecipeInstance[] = [];
const recipeInstances: Omit<RecipeInstance, 'portModulo' | 'portGap'>[] = [];

const basePort = 3001;

recipes.forEach((recipe, i) => {
recipes.forEach(recipe => {
recipe.versions.forEach(version => {
const dependencyOverrides =
Object.keys(version.dependencyOverrides).length > 0 ? version.dependencyOverrides : undefined;
Expand All @@ -20,12 +18,19 @@ export function buildRecipeInstances(recipePaths: string[]): RecipeInstance[] {
label: `${recipe.testApplicationName}${dependencyOverridesInformationString}`,
recipe,
dependencyOverrides,
port: basePort + i,
});
});
});

return recipeInstances;
return recipeInstances
.map((instance, i) => ({ ...instance, portModulo: i, portGap: recipeInstances.length }))
.filter((_, i) => {
if (process.env.E2E_TEST_SHARD && process.env.E2E_TEST_SHARD_AMOUNT) {
return (i + Number(process.env.E2E_TEST_SHARD)) % Number(process.env.E2E_TEST_SHARD_AMOUNT) === 0;
} else {
return true;
}
});
}

function buildRecipes(recipePaths: string[]): Recipe[] {
Expand Down
12 changes: 9 additions & 3 deletions packages/e2e-tests/lib/runAllTestApps.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
/* eslint-disable no-console */
import { buildRecipeInstances } from './buildRecipeInstances';
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';

import { constructRecipeInstances } from './constructRecipeInstances';
import { buildAndTestApp } from './runTestApp';
import type { RecipeInstance, RecipeTestResult } from './types';

export async function runAllTestApps(
recipePaths: string[],
envVarsToInject: Record<string, string | undefined>,
): Promise<void> {
const maxParallel = process.env.CI ? 2 : 5;
const maxParallel = process.env.CI ? 1 : 1; // For now we are disabling parallel execution because it was causing problems (runners were too slow and timeouts happened)

const recipeInstances = buildRecipeInstances(recipePaths);
const recipeInstances = constructRecipeInstances(recipePaths);

const results = await shardPromises(
recipeInstances,
Expand All @@ -33,6 +37,8 @@ export async function runAllTestApps(

const failed = results.filter(result => result.buildFailed || result.testFailed);

fs.rmSync(path.join(os.tmpdir(), 'e2e-test-yarn-caches'), { force: true, recursive: true });

if (failed.length) {
console.log(`${failed.length} test(s) failed.`);
process.exit(1);
Expand Down
5 changes: 3 additions & 2 deletions packages/e2e-tests/lib/runTestApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export async function buildAndTestApp(
recipeInstance: RecipeInstance,
envVarsToInject: Record<string, string | undefined>,
): Promise<RecipeTestResult> {
const { recipe, port } = recipeInstance;
const { recipe, portModulo, portGap } = recipeInstance;
const recipeDirname = path.dirname(recipe.path);

const targetDir = path.join(TMP_DIR, `${recipe.testApplicationName}-${tmpDirCount++}`);
Expand All @@ -24,7 +24,8 @@ export async function buildAndTestApp(

const env: Env = {
...envVarsToInject,
PORT: port.toString(),
PORT_MODULO: portModulo.toString(),
PORT_GAP: portGap.toString(),
};

try {
Expand Down
19 changes: 15 additions & 4 deletions packages/e2e-tests/lib/testApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import { DEFAULT_TEST_TIMEOUT_SECONDS } from './constants';
import type { Env, RecipeInstance, TestDef, TestResult } from './types';
import { spawnAsync } from './utils';
import { prefixObjectKeys, spawnAsync } from './utils';

export async function testApp(appDir: string, recipeInstance: RecipeInstance, env: Env): Promise<TestResult[]> {
const { recipe } = recipeInstance;
Expand All @@ -15,17 +15,28 @@ export async function testApp(appDir: string, recipeInstance: RecipeInstance, en
return results;
}

async function runTest(appDir: string, recipeInstance: RecipeInstance, test: TestDef, env: Env): Promise<TestResult> {
async function runTest(
appDir: string,
recipeInstance: RecipeInstance,
test: TestDef,
envVars: Env,
): Promise<TestResult> {
const { recipe, label } = recipeInstance;
console.log(`Running test command for test application "${label}", test "${test.testName}"`);

const env = {
...process.env,
...envVars,
};

const testResult = await spawnAsync(test.testCommand, {
cwd: appDir,
timeout: (recipe.testTimeoutSeconds ?? DEFAULT_TEST_TIMEOUT_SECONDS) * 1000,
env: {
...process.env,
...env,
} as unknown as NodeJS.ProcessEnv,
...prefixObjectKeys(env, 'NEXT_PUBLIC_'),
...prefixObjectKeys(env, 'REACT_APP_'),
},
});

if (testResult.error) {
Expand Down
3 changes: 2 additions & 1 deletion packages/e2e-tests/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ export interface RecipeInstance {
label: string;
recipe: Recipe;
dependencyOverrides?: DependencyOverrides;
port: number;
portModulo: number;
portGap: number;
}

export interface RecipeTestResult extends RecipeInstance {
Expand Down
10 changes: 10 additions & 0 deletions packages/e2e-tests/lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,13 @@ export function spawnAsync(
}
});
}

export function prefixObjectKeys(
obj: Record<string, string | undefined>,
prefix: string,
): Record<string, string | undefined> {
return Object.keys(obj).reduce<Record<string, string | undefined>>((result, key) => {
result[prefix + key] = obj[key];
return result;
}, {});
}
1 change: 1 addition & 0 deletions packages/e2e-tests/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ async function run(): Promise<void> {
const envVarsToInject = {
REACT_APP_E2E_TEST_DSN: process.env.E2E_TEST_DSN,
NEXT_PUBLIC_E2E_TEST_DSN: process.env.E2E_TEST_DSN,
BASE_PORT: '27496', // just some random port
};

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,9 @@
"version": "0.1.0",
"private": true,
"scripts": {
"dev": "next dev",
"build": "next build",
"start": "next start",
"lint": "next lint",
"test": "test:prod && test:dev",
"test:prod": "TEST_MODE=prod playwright test",
"test:dev": "TEST_MODE=dev playwright test"
"test:prod": "TEST_ENV=prod playwright test",
"test:dev": "TEST_ENV=dev playwright test"
},
"dependencies": {
"@next/font": "13.0.7",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
import type { PlaywrightTestConfig } from '@playwright/test';
import { devices } from '@playwright/test';

const testEnv = process.env.TEST_ENV;

if (!testEnv) {
throw new Error('No test env defined');
}

const port = Number(process.env.BASE_PORT) + Number(process.env.PORT_MODULO);

/**
* See https://playwright.dev/docs/test-configuration.
*/
Expand Down Expand Up @@ -59,8 +67,8 @@ const config: PlaywrightTestConfig = {

/* Run your local dev server before starting the tests */
webServer: {
command: process.env.TEST_MODE === 'prod' ? 'yarn start' : 'yarn dev',
port: process.env.PORT ? parseInt(process.env.PORT) : 3000,
command: testEnv === 'development' ? `yarn next dev -p ${port}` : `yarn next start -p ${port}`,
port,
},
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"$schema": "../../test-recipe-schema.json",
"testApplicationName": "create-next-app",
"buildCommand": "yarn install --network-concurrency 1 && npx playwright install && yarn build",
"buildCommand": "yarn install && npx playwright install && yarn build",
"tests": [
{
"testName": "Playwright tests - Prod Mode",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"$schema": "../../test-recipe-schema.json",
"testApplicationName": "create-react-app",
"buildCommand": "yarn install --network-concurrency 1 && yarn build",
"buildCommand": "yarn install && yarn build",
"tests": []
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@
"version": "0.1.0",
"private": true,
"scripts": {
"dev": "next dev",
"build": "next build",
"start": "next start",
"lint": "next lint",
"test:prod": "TEST_ENV=production playwright test",
"test:dev": "TEST_ENV=development playwright test"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ if (!testEnv) {
throw new Error('No test env defined');
}

const port = process.env.PORT ? parseInt(process.env.PORT) : 3000;
const port = Number(process.env.BASE_PORT) + Number(process.env.PORT_MODULO);

/**
* See https://playwright.dev/docs/test-configuration.
Expand Down Expand Up @@ -55,12 +55,12 @@ const config: PlaywrightTestConfig = {
/* Run your local dev server before starting the tests */
webServer: [
{
command: testEnv === 'development' ? 'yarn dev' : 'yarn start',
command: testEnv === 'development' ? `yarn next dev -p ${port}` : `yarn next start -p ${port}`,
port,
},
{
command: 'yarn ts-node-script start-event-proxy.ts',
port: 27496,
port: Number(process.env.BASE_PORT) + Number(process.env.PORT_MODULO) + Number(process.env.PORT_GAP),
},
],
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import * as Sentry from '@sentry/nextjs';

Sentry.init({
dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN,
tunnel: 'http://localhost:27496/', // proxy server
tunnel: `http://localhost:${
Number(process.env.NEXT_PUBLIC_BASE_PORT) +
Number(process.env.NEXT_PUBLIC_PORT_MODULO) +
Number(process.env.NEXT_PUBLIC_PORT_GAP)
}/`, // proxy server
tracesSampleRate: 1.0,
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import * as Sentry from '@sentry/nextjs';

Sentry.init({
dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN,
tunnel: 'http://localhost:27496/', // proxy server
tunnel: `http://localhost:${
Number(process.env.NEXT_PUBLIC_BASE_PORT) +
Number(process.env.NEXT_PUBLIC_PORT_MODULO) +
Number(process.env.NEXT_PUBLIC_PORT_GAP)
}/`, // proxy server
tracesSampleRate: 1.0,
});
Loading