Skip to content

fix(build): Fix temporary sucrase CI jobs #5021

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 4 commits into from
Apr 29, 2022
Merged
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
96 changes: 34 additions & 62 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -105,45 +105,6 @@ jobs:
# `job_build` can't see `job_install_deps` and what it returned)
dependency_cache_key: ${{ needs.job_install_deps.outputs.dependency_cache_key }}

# This isn't a full `yarn build` using sucrase - it's just the cache from the normal build, with `build/cjs` and
# `build/esm` overwritten by sucrase. This way we don't need to worry about all of the other random stuff which
# packages build, because it will already be there.
job_build_with_sucrase:
name: Sucrase Build
needs: [job_install_deps, job_build]
runs-on: ubuntu-latest
timeout-minutes: 20
steps:
- name: Check out current commit (${{ env.HEAD_COMMIT }})
uses: actions/checkout@v2
with:
ref: ${{ env.HEAD_COMMIT }}
- name: Set up Node
uses: actions/setup-node@v1
- name: Check dependency cache
uses: actions/cache@v2
with:
path: ${{ env.CACHED_DEPENDENCY_PATHS }}
key: ${{ needs.job_install_deps.outputs.dependency_cache_key }}
- name: Check tsc build cache
uses: actions/cache@v2
with:
path: ${{ env.CACHED_BUILD_PATHS }}
key: ${{ env.BUILD_CACHE_KEY }}
- name: Check sucrase build cache
uses: actions/cache@v2
id: cache_built_sucrase_packages
with:
path: ${{ env.CACHED_BUILD_PATHS }}
key: ${{ env.BUILD_CACHE_KEY }}-sucrase
- name: Build packages with sucrase
if: steps.cache_built_sucrase_packages.outputs.cache-hit == ''
run: 'yarn build:rollup'
outputs:
# this needs to be passed on, because the `needs` context only looks at direct ancestors (so steps which depend on
# `job_build` can't see `job_install_deps` and what it returned)
dependency_cache_key: ${{ needs.job_install_deps.outputs.dependency_cache_key }}

job_size_check:
name: Size Check
needs: job_build
Expand Down Expand Up @@ -295,6 +256,8 @@ jobs:
NODE_VERSION: ${{ matrix.node }}
run: |
[[ $NODE_VERSION == 8 ]] && yarn add --dev --ignore-engines --ignore-scripts --ignore-workspace-root-check [email protected]
# TODO remove this when we switch to sucrase
yarn ts-node scripts/sucrase-test-hack.ts
yarn test-ci
- name: Compute test coverage
uses: codecov/codecov-action@v1
Expand Down Expand Up @@ -534,7 +497,7 @@ jobs:

job_unit_test_sucrase:
name: Sucrase Test (Node ${{ matrix.node }})
needs: job_build_with_sucrase
needs: job_build
continue-on-error: true
timeout-minutes: 30
runs-on: ubuntu-latest
Expand All @@ -554,24 +517,26 @@ jobs:
uses: actions/cache@v2
with:
path: ${{ env.CACHED_DEPENDENCY_PATHS }}
key: ${{ needs.job_build_with_sucrase.outputs.dependency_cache_key }}
key: ${{ needs.job_build.outputs.dependency_cache_key }}
- name: Check build cache
uses: actions/cache@v2
with:
path: ${{ env.CACHED_BUILD_PATHS }}
key: ${{ env.BUILD_CACHE_KEY }}-sucrase
key: ${{ env.BUILD_CACHE_KEY }}
- name: Run tests
env:
NODE_VERSION: ${{ matrix.node }}
SUCRASE: true
run: |
[[ $NODE_VERSION == 8 ]] && yarn add --dev --ignore-engines --ignore-scripts --ignore-workspace-root-check [email protected]
[[ $NODE_VERSION == 8 ]] && yarn add --dev --ignore-engines --ignore-scripts --ignore-workspace-root-check [email protected] [email protected] @rollup/[email protected] [email protected] [email protected]
yarn ts-node scripts/sucrase-test-hack.ts
yarn test-ci
- name: Compute test coverage
uses: codecov/codecov-action@v1

job_nextjs_integration_test_sucrase:
name: Sucrase Test @sentry/nextjs on (Node ${{ matrix.node }})
needs: job_build_with_sucrase
needs: job_build
continue-on-error: true
timeout-minutes: 30
runs-on: ubuntu-latest
Expand All @@ -591,24 +556,25 @@ jobs:
uses: actions/cache@v2
with:
path: ${{ env.CACHED_DEPENDENCY_PATHS }}
key: ${{ needs.job_build_with_sucrase.outputs.dependency_cache_key }}
key: ${{ needs.job_build.outputs.dependency_cache_key }}
- name: Check build cache
uses: actions/cache@v2
with:
path: ${{ env.CACHED_BUILD_PATHS }}
key: ${{ env.BUILD_CACHE_KEY }}-sucrase
key: ${{ env.BUILD_CACHE_KEY }}
- name: Run tests
env:
NODE_VERSION: ${{ matrix.node }}
run: |
yarn ts-node scripts/sucrase-test-hack.ts
cd packages/nextjs
yarn test:integration

# Ember tests are separate from the rest because they are the slowest part of the test suite, and making them a
# separate job allows them to run in parallel with the other tests.
job_ember_tests_sucrase:
name: Sucrase Test @sentry/ember
needs: job_build_with_sucrase
needs: job_build
continue-on-error: true
timeout-minutes: 30
runs-on: ubuntu-latest
Expand All @@ -634,20 +600,22 @@ jobs:
uses: actions/cache@v2
with:
path: ${{ env.CACHED_DEPENDENCY_PATHS }}
key: ${{ needs.job_build_with_sucrase.outputs.dependency_cache_key }}
key: ${{ needs.job_build.outputs.dependency_cache_key }}
- name: Check build cache
uses: actions/cache@v2
with:
path: ${{ env.CACHED_BUILD_PATHS }}
key: ${{ env.BUILD_CACHE_KEY }}-sucrase
key: ${{ env.BUILD_CACHE_KEY }}
- name: Run Ember tests
run: yarn test --scope=@sentry/ember
run: |
yarn ts-node scripts/sucrase-test-hack.ts
yarn test --scope=@sentry/ember
- name: Compute test coverage
uses: codecov/codecov-action@v1

job_browser_playwright_tests_sucrase:
name: Sucrase Playwright - ${{ (matrix.tracing_only && 'Browser + Tracing') || 'Browser' }} (${{ matrix.bundle }})
needs: job_build_with_sucrase
needs: job_build
runs-on: ubuntu-latest
strategy:
matrix:
Expand Down Expand Up @@ -677,24 +645,25 @@ jobs:
uses: actions/cache@v2
with:
path: ${{ env.CACHED_DEPENDENCY_PATHS }}
key: ${{ needs.job_build_with_sucrase.outputs.dependency_cache_key }}
key: ${{ needs.job_build.outputs.dependency_cache_key }}
- name: Check build cache
uses: actions/cache@v2
with:
path: ${{ env.CACHED_BUILD_PATHS }}
key: ${{ env.BUILD_CACHE_KEY }}-sucrase
key: ${{ env.BUILD_CACHE_KEY }}
- name: Run Playwright tests
env:
PW_BUNDLE: ${{ matrix.bundle }}
PW_TRACING_ONLY: ${{ matrix.tracing_only }}
run: |
yarn ts-node scripts/sucrase-test-hack.ts
cd packages/integration-tests
yarn run playwright install-deps webkit
yarn test:ci

job_browser_integration_tests_sucrase:
name: Sucrase Old Browser Integration Tests (${{ matrix.browser }})
needs: job_build_with_sucrase
needs: job_build
runs-on: ubuntu-latest
timeout-minutes: 10
continue-on-error: true
Expand All @@ -715,23 +684,24 @@ jobs:
uses: actions/cache@v2
with:
path: ${{ env.CACHED_DEPENDENCY_PATHS }}
key: ${{ needs.job_build_with_sucrase.outputs.dependency_cache_key }}
key: ${{ needs.job_build.outputs.dependency_cache_key }}
- name: Check build cache
uses: actions/cache@v2
with:
path: ${{ env.CACHED_BUILD_PATHS }}
key: ${{ env.BUILD_CACHE_KEY }}-sucrase
key: ${{ env.BUILD_CACHE_KEY }}
- name: Run integration tests
env:
KARMA_BROWSER: ${{ matrix.browser }}
run: |
yarn ts-node scripts/sucrase-test-hack.ts
cd packages/browser
[[ $KARMA_BROWSER == WebkitHeadless ]] && yarn run playwright install-deps webkit
yarn test:integration

job_browser_build_tests_sucrase:
name: Sucrase Browser Build Tests
needs: job_build_with_sucrase
needs: job_build
runs-on: ubuntu-latest
timeout-minutes: 5
continue-on-error: true
Expand All @@ -748,14 +718,15 @@ jobs:
uses: actions/cache@v2
with:
path: ${{ env.CACHED_DEPENDENCY_PATHS }}
key: ${{ needs.job_build_with_sucrase.outputs.dependency_cache_key }}
key: ${{ needs.job_build.outputs.dependency_cache_key }}
- name: Check build cache
uses: actions/cache@v2
with:
path: ${{ env.CACHED_BUILD_PATHS }}
key: ${{ env.BUILD_CACHE_KEY }}-sucrase
key: ${{ env.BUILD_CACHE_KEY }}
- name: Run browser build tests
run: |
yarn ts-node scripts/sucrase-test-hack.ts
cd packages/browser
yarn test:package
- name: Run utils build tests
Expand All @@ -765,7 +736,7 @@ jobs:

job_node_integration_tests_sucrase:
name: Sucrase Node SDK Integration Tests (${{ matrix.node }})
needs: job_build_with_sucrase
needs: job_build
runs-on: ubuntu-latest
timeout-minutes: 10
continue-on-error: true
Expand All @@ -785,15 +756,16 @@ jobs:
uses: actions/cache@v2
with:
path: ${{ env.CACHED_DEPENDENCY_PATHS }}
key: ${{ needs.job_build_with_sucrase.outputs.dependency_cache_key }}
key: ${{ needs.job_build.outputs.dependency_cache_key }}
- name: Check build cache
uses: actions/cache@v2
with:
path: ${{ env.CACHED_BUILD_PATHS }}
key: ${{ env.BUILD_CACHE_KEY }}-sucrase
key: ${{ env.BUILD_CACHE_KEY }}
- name: Run integration tests
env:
NODE_VERSION: ${{ matrix.node }}
run: |
yarn ts-node scripts/sucrase-test-hack.ts
cd packages/node-integration-tests
yarn test
18 changes: 15 additions & 3 deletions rollup/npmHelpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ export function makeBaseNPMConfig(options = {}) {
const nodeResolvePlugin = makeNodeResolvePlugin();
const sucrasePlugin = makeSucrasePlugin();

return {
// return {
const config = {
input: entrypoints,

output: {
Expand All @@ -32,10 +33,10 @@ export function makeBaseNPMConfig(options = {}) {
sourcemap: true,

// output individual files rather than one big bundle
preserveModules: true,
// preserveModules: true,

// any wrappers or helper functions generated by rollup can use ES6 features
generatedCode: 'es2015',
// generatedCode: 'es2015',

// don't add `"use strict"` to the top of cjs files
strict: false,
Expand Down Expand Up @@ -78,6 +79,17 @@ export function makeBaseNPMConfig(options = {}) {
// treeshake: 'smallest',
treeshake: false,
};

// temporary hack for testing sucrase bundles before we switch over
if (!process.version.startsWith('v8')) {
console.log('Doing normal preserveModules');
config.output.preserveModules = true;
} else {
console.log('Doing node 8 preserveModules');
config.preserveModules = true;
}

return config;
}

export function makeNPMConfigVariants(baseConfig) {
Expand Down
49 changes: 49 additions & 0 deletions scripts/sucrase-test-hack.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// A temporary hack to use sucrase versions of packages for testing in CI.

import * as childProcess from 'child_process';
import * as fs from 'fs';
import * as path from 'path';

function run(cmd: string, options?: childProcess.ExecSyncOptions): unknown {
return childProcess.execSync(cmd, { stdio: 'inherit', ...options });
}

const ignorePackages = process.version.startsWith('v8')
? [
'@sentry/ember',
'@sentry-internal/eslint-plugin-sdk',
'@sentry/react',
'@sentry/wasm',
'@sentry/gatsby',
'@sentry/serverless',
'@sentry/nextjs',
'@sentry/angular',
]
: ['@sentry/serverless'];

// clear current builds and rebuild with rollup/sucrase (this way, all of the extra, random stuff which gets built in
// the main build job remains, and only the part affected by this project gets overwritten)
if (process.env.SUCRASE) {
// just to be super sure
fs.readdirSync(path.join(process.cwd(), 'packages')).forEach(dir => {
if (fs.existsSync(path.join(process.cwd(), 'packages', dir, 'build', 'npm'))) {
run(`rm -rf packages/${dir}/build/npm/cjs`);
run(`rm -rf packages/${dir}/build/npm/esm`);
} else if (fs.existsSync(path.join(process.cwd(), 'packages', dir, 'build', 'cjs'))) {
run(`rm -rf packages/${dir}/build/cjs`);
run(`rm -rf packages/${dir}/build/esm`);
}
});

// rebuild the packages we're going to test with rollup/sucrase
run(`yarn build:rollup ${ignorePackages.map(dep => `--ignore="${dep}"`).join(' ')}`);
}
// if we're in tsc-land, rebuild using es5 - temporary until switch to sucrase
else {
const baseTSConfigPath = 'packages/typescript/tsconfig.json';
fs.writeFileSync(
baseTSConfigPath,
String(fs.readFileSync(baseTSConfigPath)).replace('"target": "es6"', '"target": "es5"'),
);
run(`yarn build:dev ${ignorePackages.map(dep => `--ignore="${dep}"`).join(' ')}`);
}
7 changes: 0 additions & 7 deletions scripts/test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { spawnSync } from 'child_process';
import * as fs from 'fs';
import { join } from 'path';

function run(cmd: string, cwd: string = '') {
Expand Down Expand Up @@ -48,12 +47,6 @@ if (nodeMajorVersion <= 10) {
// against a single version of node, but in the short run, this at least allows us to not be blocked by the
// failures.)
run('rm -rf packages/tracing/test/browser');

// TODO Pull this out once we switch to sucrase builds
// Recompile as es5, so as not to have to fix a compatibility problem that will soon be moot
const baseTSConfig = 'packages/typescript/tsconfig.json';
fs.writeFileSync(baseTSConfig, String(fs.readFileSync(baseTSConfig)).replace('"target": "es6"', '"target": "es5"'));
run(`yarn build:dev ${ignorePackages.map(dep => `--ignore="${dep}"`).join(' ')}`);
}
// Node 10
else {
Expand Down