diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2eb36d6ef53b..313b93724178 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -6,28 +6,80 @@ on: - release/** pull_request: +env: + CACHED_DEPENDENCY_PATHS: | + ${{ github.workspace }}/node_modules + ${{ github.workspace }}/packages/**/node_modules + + # DEPENDENCY_CACHE_KEY: can't be set here because we don't have access to yarn.lock + + CACHED_BUILD_PATHS: | + ${{ github.workspace }}/packages/**/build + ${{ github.workspace }}/packages/**/dist + ${{ github.workspace }}/packages/**/esm + ${{ github.workspace }}/packages/serverless/dist-awslambda-layer/*.zip + + BUILD_CACHE_KEY: ${{ github.sha }} + jobs: + job_install_deps: + name: Install Dependencies + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - name: Check out current commit (${{ github.sha }}) + uses: actions/checkout@v2 + - name: Set up Node + uses: actions/setup-node@v1 + # we use a hash of yarn.lock as our cache key, because if it hasn't changed, our dependencies haven't changed, + # so no need to reinstall them + - name: Compute dependency cache key + id: compute_lockfile_hash + run: echo "::set-output name=hash::${{ hashFiles('yarn.lock') }}" + - name: Check dependency cache + uses: actions/cache@v2 + id: cache_dependencies + with: + path: ${{ env.CACHED_DEPENDENCY_PATHS }} + key: ${{ steps.compute_lockfile_hash.outputs.hash }} + - name: Install dependencies + if: steps.cache_dependencies.outputs.cache-hit == '' + run: yarn install + outputs: + dependency_cache_key: ${{ steps.compute_lockfile_hash.outputs.hash }} + job_build: name: Build + needs: job_install_deps runs-on: ubuntu-latest timeout-minutes: 15 steps: - - uses: actions/checkout@v2 - - uses: actions/setup-node@v1 - - uses: actions/cache@v2 + - name: Check out current commit (${{ github.sha }}) + uses: actions/checkout@v2 + - name: Set up Node + uses: actions/setup-node@v1 + - name: Check dependency cache + uses: actions/cache@v2 with: - path: | - ${{ github.workspace }}/node_modules - ${{ github.workspace }}/packages/**/node_modules - ${{ github.workspace }}/packages/**/build - ${{ github.workspace }}/packages/**/dist - ${{ github.workspace }}/packages/**/esm - ${{ github.workspace }}/packages/serverless/dist-awslambda-layer/*.zip - key: ${{ github.sha }} - - name: Install - run: yarn install - - name: Build + path: ${{ env.CACHED_DEPENDENCY_PATHS }} + key: ${{ needs.job_install_deps.outputs.dependency_cache_key }} + - name: Check build cache + uses: actions/cache@v2 + id: cache_built_packages + with: + path: ${{ env.CACHED_BUILD_PATHS }} + key: ${{ env.BUILD_CACHE_KEY }} + - name: Build packages + # Under normal circumstances, using the git SHA as a cache key, there shouldn't ever be a cache hit on the built + # packages, and so `yarn build` should always run. This `if` check is therefore only there for testing CI issues + # where the built packages are beside the point. In that case, you can change `BUILD_CACHE_KEY` (at the top of + # this file) to a constant and skip rebuilding all of the packages each time CI runs. + if: steps.cache_built_packages.outputs.cache-hit == '' run: yarn build + 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 @@ -36,19 +88,22 @@ jobs: runs-on: ubuntu-latest if: ${{ github.head_ref }} steps: - - uses: actions/checkout@v2 - - uses: actions/setup-node@v1 - - uses: actions/cache@v2 + - name: Check out current commit (${{ github.sha }}) + uses: actions/checkout@v2 + - name: Set up Node + uses: actions/setup-node@v1 + - name: Check dependency cache + uses: actions/cache@v2 with: - path: | - ${{ github.workspace }}/node_modules - ${{ github.workspace }}/packages/**/node_modules - ${{ github.workspace }}/packages/**/build - ${{ github.workspace }}/packages/**/dist - ${{ github.workspace }}/packages/**/esm - ${{ github.workspace }}/packages/serverless/dist-awslambda-layer/*.zip - key: ${{ github.sha }} - - uses: andresz1/size-limit-action@v1.4.0 + path: ${{ env.CACHED_DEPENDENCY_PATHS }} + 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 }} + - name: Check bundle sizes + uses: andresz1/size-limit-action@v1.4.0 with: github_token: ${{ secrets.GITHUB_TOKEN }} skip_step: build @@ -59,20 +114,21 @@ jobs: timeout-minutes: 10 runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 - - uses: actions/setup-node@v1 - - uses: actions/cache@v2 + - name: Check out current commit (${{ github.sha }}) + uses: actions/checkout@v2 + - name: Set up Node + uses: actions/setup-node@v1 + - name: Check dependency cache + uses: actions/cache@v2 with: - path: | - ${{ github.workspace }}/node_modules - ${{ github.workspace }}/packages/**/node_modules - ${{ github.workspace }}/packages/**/build - ${{ github.workspace }}/packages/**/dist - ${{ github.workspace }}/packages/**/esm - ${{ github.workspace }}/packages/serverless/dist-awslambda-layer/*.zip - key: ${{ github.sha }} - - run: yarn install - - name: Run Linter + path: ${{ env.CACHED_DEPENDENCY_PATHS }} + 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 }} + - name: Run linter run: yarn lint job_unit_test: @@ -85,47 +141,85 @@ jobs: matrix: node: [6, 8, 10, 12, 14] steps: - - uses: actions/checkout@v2 - - uses: actions/setup-node@v1 + - name: Check out current commit (${{ github.sha }}) + uses: actions/checkout@v2 + - name: Set up Node + uses: actions/setup-node@v1 with: node-version: ${{ matrix.node }} - - uses: actions/cache@v2 + - name: Check dependency cache + uses: actions/cache@v2 with: - path: | - ${{ github.workspace }}/node_modules - ${{ github.workspace }}/packages/**/node_modules - ${{ github.workspace }}/packages/**/build - ${{ github.workspace }}/packages/**/dist - ${{ github.workspace }}/packages/**/esm - ${{ github.workspace }}/packages/serverless/dist-awslambda-layer/*.zip - key: ${{ github.sha }} - - name: Unit Tests + path: ${{ env.CACHED_DEPENDENCY_PATHS }} + 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 }} + - name: Run tests env: NODE_VERSION: ${{ matrix.node }} run: ./scripts/test.sh - - uses: codecov/codecov-action@v1 + - name: Compute test coverage + uses: codecov/codecov-action@v1 + + # 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: + name: Test @sentry/ember + needs: job_build + continue-on-error: true + timeout-minutes: 30 + runs-on: ubuntu-latest + steps: + - name: Check out current commit (${{ github.sha }}) + uses: actions/checkout@v2 + - name: Set up Node + uses: actions/setup-node@v1 + with: + # The only danger with Ember in terms of Node versions is that the build tool, ember-cli, won't work if Node + # is too old. Since Oct 2019, Node 10 has been the oldest version supported by ember-cli, so test against + # that. If it passes, newer versions of Node should also be fine. This saves us from having to run the Ember + # tests in our Node matrix above. + node-version: '10' + - name: Check dependency cache + uses: actions/cache@v2 + with: + path: ${{ env.CACHED_DEPENDENCY_PATHS }} + 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 }} + - name: Run Ember tests + run: yarn test --scope=@sentry/ember + - name: Compute test coverage + uses: codecov/codecov-action@v1 job_artifacts: - name: Artifacts Upload + name: Upload Artifacts needs: job_build runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 - - uses: actions/setup-node@v1 - - uses: actions/cache@v2 + - name: Check out current commit (${{ github.sha }}) + uses: actions/checkout@v2 + - name: Set up Node + uses: actions/setup-node@v1 + - name: Check dependency cache + uses: actions/cache@v2 with: - path: | - ${{ github.workspace }}/node_modules - ${{ github.workspace }}/packages/**/node_modules - ${{ github.workspace }}/packages/**/build - ${{ github.workspace }}/packages/**/dist - ${{ github.workspace }}/packages/**/esm - ${{ github.workspace }}/packages/serverless/dist-awslambda-layer/*.zip - key: ${{ github.sha }} + path: ${{ env.CACHED_DEPENDENCY_PATHS }} + 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 }} - name: Pack run: yarn pack:changed - - run: yarn install - - name: Archive Artifacts + - name: Archive artifacts uses: actions/upload-artifact@v2 with: name: ${{ github.sha }} @@ -146,20 +240,21 @@ jobs: # if: startsWith(github.ref, 'refs/heads/release/') if: false steps: - - uses: actions/checkout@v2 - - uses: actions/setup-node@v1 - - uses: actions/cache@v2 + - name: Check out current commit (${{ github.sha }}) + uses: actions/checkout@v2 + - name: Set up Node + uses: actions/setup-node@v1 + - name: Check dependency cache + uses: actions/cache@v2 with: - path: | - ${{ github.workspace }}/node_modules - ${{ github.workspace }}/packages/**/node_modules - ${{ github.workspace }}/packages/**/build - ${{ github.workspace }}/packages/**/dist - ${{ github.workspace }}/packages/**/esm - ${{ github.workspace }}/packages/serverless/dist-awslambda-layer/*.zip - key: ${{ github.sha }} - - run: yarn install - - name: Integration Tests + path: ${{ env.CACHED_DEPENDENCY_PATHS }} + 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 }} + - name: Run integration tests env: BROWSERSTACK_USERNAME: ${{ secrets.BROWSERSTACK_USERNAME }} BROWSERSTACK_ACCESS_KEY: ${{ secrets.BROWSERSTACK_ACCESS_KEY }} diff --git a/packages/tracing/test/hub.test.ts b/packages/tracing/test/hub.test.ts index a9beed2201f7..edb2c22af367 100644 --- a/packages/tracing/test/hub.test.ts +++ b/packages/tracing/test/hub.test.ts @@ -10,7 +10,7 @@ import { BrowserTracing } from '../src/browser/browsertracing'; import { addExtensionMethods } from '../src/hubextensions'; import { Transaction } from '../src/transaction'; import { extractTraceparentData, TRACEPARENT_REGEXP } from '../src/utils'; -import { addDOMPropertiesToGlobal, getSymbolObjectKeyByName } from './testutils'; +import { addDOMPropertiesToGlobal, getSymbolObjectKeyByName, testOnlyIfNodeVersionAtLeast } from './testutils'; addExtensionMethods(); @@ -309,77 +309,89 @@ describe('Hub', () => { expect(child.sampled).toBe(transaction.sampled); }); - it('should propagate positive sampling decision to child transactions in XHR header', () => { - const hub = new Hub( - new BrowserClient({ - dsn: 'https://1231@dogs.are.great/1121', - tracesSampleRate: 1, - integrations: [new BrowserTracing()], - }), - ); - jest.spyOn(hubModule, 'getCurrentHub').mockReturnValue(hub); - - const transaction = hub.startTransaction({ name: 'dogpark' }); - hub.configureScope(scope => { - scope.setSpan(transaction); - }); - - const request = new XMLHttpRequest(); - request.open('GET', '/chase-partners'); - - // mock a response having been received successfully (we have to do it in this roundabout way because readyState - // is readonly and changing it doesn't trigger a readystatechange event) - Object.defineProperty(request, 'readyState', { value: 4 }); - request.dispatchEvent(new Event('readystatechange')); - - // this looks weird, it's true, but it's really just `request.impl.flag.requestHeaders` - it's just that the - // `impl` key is a symbol rather than a string, and therefore needs to be referred to by reference rather than - // value - const headers = (request as any)[getSymbolObjectKeyByName(request, 'impl') as symbol].flag.requestHeaders; - - // check that sentry-trace header is added to request - expect(headers).toEqual(expect.objectContaining({ 'sentry-trace': expect.stringMatching(TRACEPARENT_REGEXP) })); - - // check that sampling decision is passed down correctly - expect(transaction.sampled).toBe(true); - expect(extractTraceparentData(headers['sentry-trace'])!.parentSampled).toBe(true); - }); - - it('should propagate negative sampling decision to child transactions in XHR header', () => { - const hub = new Hub( - new BrowserClient({ - dsn: 'https://1231@dogs.are.great/1121', - tracesSampleRate: 1, - integrations: [new BrowserTracing()], - }), - ); - jest.spyOn(hubModule, 'getCurrentHub').mockReturnValue(hub); - - const transaction = hub.startTransaction({ name: 'dogpark', sampled: false }); - hub.configureScope(scope => { - scope.setSpan(transaction); - }); - - const request = new XMLHttpRequest(); - request.open('GET', '/chase-partners'); - - // mock a response having been received successfully (we have to do it in this roundabout way because readyState - // is readonly and changing it doesn't trigger a readystatechange event) - Object.defineProperty(request, 'readyState', { value: 4 }); - request.dispatchEvent(new Event('readystatechange')); - - // this looks weird, it's true, but it's really just `request.impl.flag.requestHeaders` - it's just that the - // `impl` key is a symbol rather than a string, and therefore needs to be referred to by reference rather than - // value - const headers = (request as any)[getSymbolObjectKeyByName(request, 'impl') as symbol].flag.requestHeaders; - - // check that sentry-trace header is added to request - expect(headers).toEqual(expect.objectContaining({ 'sentry-trace': expect.stringMatching(TRACEPARENT_REGEXP) })); - - // check that sampling decision is passed down correctly - expect(transaction.sampled).toBe(false); - expect(extractTraceparentData(headers['sentry-trace'])!.parentSampled).toBe(false); - }); + // TODO the way we dig out the headers to test them doesn't work on Node < 10 + testOnlyIfNodeVersionAtLeast(10)( + 'should propagate positive sampling decision to child transactions in XHR header', + () => { + const hub = new Hub( + new BrowserClient({ + dsn: 'https://1231@dogs.are.great/1121', + tracesSampleRate: 1, + integrations: [new BrowserTracing()], + }), + ); + jest.spyOn(hubModule, 'getCurrentHub').mockReturnValue(hub); + + const transaction = hub.startTransaction({ name: 'dogpark' }); + hub.configureScope(scope => { + scope.setSpan(transaction); + }); + + const request = new XMLHttpRequest(); + request.open('GET', '/chase-partners'); + + // mock a response having been received successfully (we have to do it in this roundabout way because readyState + // is readonly and changing it doesn't trigger a readystatechange event) + Object.defineProperty(request, 'readyState', { value: 4 }); + request.dispatchEvent(new Event('readystatechange')); + + // this looks weird, it's true, but it's really just `request.impl.flag.requestHeaders` - it's just that the + // `impl` key is a symbol rather than a string, and therefore needs to be referred to by reference rather than + // value + const headers = (request as any)[getSymbolObjectKeyByName(request, 'impl') as symbol].flag.requestHeaders; + + // check that sentry-trace header is added to request + expect(headers).toEqual( + expect.objectContaining({ 'sentry-trace': expect.stringMatching(TRACEPARENT_REGEXP) }), + ); + + // check that sampling decision is passed down correctly + expect(transaction.sampled).toBe(true); + expect(extractTraceparentData(headers['sentry-trace'])!.parentSampled).toBe(true); + }, + ); + + // TODO the way we dig out the headers to test them doesn't work on Node < 10 + testOnlyIfNodeVersionAtLeast(10)( + 'should propagate negative sampling decision to child transactions in XHR header', + () => { + const hub = new Hub( + new BrowserClient({ + dsn: 'https://1231@dogs.are.great/1121', + tracesSampleRate: 1, + integrations: [new BrowserTracing()], + }), + ); + jest.spyOn(hubModule, 'getCurrentHub').mockReturnValue(hub); + + const transaction = hub.startTransaction({ name: 'dogpark', sampled: false }); + hub.configureScope(scope => { + scope.setSpan(transaction); + }); + + const request = new XMLHttpRequest(); + request.open('GET', '/chase-partners'); + + // mock a response having been received successfully (we have to do it in this roundabout way because readyState + // is readonly and changing it doesn't trigger a readystatechange event) + Object.defineProperty(request, 'readyState', { value: 4 }); + request.dispatchEvent(new Event('readystatechange')); + + // this looks weird, it's true, but it's really just `request.impl.flag.requestHeaders` - it's just that the + // `impl` key is a symbol rather than a string, and therefore needs to be referred to by reference rather than + // value + const headers = (request as any)[getSymbolObjectKeyByName(request, 'impl') as symbol].flag.requestHeaders; + + // check that sentry-trace header is added to request + expect(headers).toEqual( + expect.objectContaining({ 'sentry-trace': expect.stringMatching(TRACEPARENT_REGEXP) }), + ); + + // check that sampling decision is passed down correctly + expect(transaction.sampled).toBe(false); + expect(extractTraceparentData(headers['sentry-trace'])!.parentSampled).toBe(false); + }, + ); it('should propagate positive sampling decision to child transactions in fetch header', () => { // TODO diff --git a/packages/tracing/test/index.bundle.test.ts b/packages/tracing/test/index.bundle.test.ts index 8a9070a12844..d4cae383ab7f 100644 --- a/packages/tracing/test/index.bundle.test.ts +++ b/packages/tracing/test/index.bundle.test.ts @@ -1,7 +1,9 @@ import { Integrations } from '../src/index.bundle'; +import { testOnlyIfNodeVersionAtLeast } from './testutils'; describe('Integrations export', () => { - it('is exported correctly', () => { + // TODO `Object.values` doesn't work on Node < 8 + testOnlyIfNodeVersionAtLeast(8)('is exported correctly', () => { Object.values(Integrations).forEach(integration => { expect(integration.id).toStrictEqual(expect.any(String)); }); diff --git a/packages/tracing/test/testutils.ts b/packages/tracing/test/testutils.ts index 0910b60ed2dd..a8f42f84b9b8 100644 --- a/packages/tracing/test/testutils.ts +++ b/packages/tracing/test/testutils.ts @@ -42,3 +42,17 @@ export function getSymbolObjectKeyByName(obj: Record, desc return matches[0] || undefined; } + +export const testOnlyIfNodeVersionAtLeast = (minVersion: number): jest.It => { + const currentNodeVersion = process.env.NODE_VERSION; + + try { + if (Number(currentNodeVersion?.split('.')[0]) < minVersion) { + return it.skip; + } + } catch (oO) { + // we can't tell, so err on the side of running the test + } + + return it; +}; diff --git a/scripts/test.sh b/scripts/test.sh index 2e9a11a171d5..7c7f5f585f88 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -1,30 +1,33 @@ #!/bin/bash set -e -source ~/.nvm/nvm.sh - -# We need this check to skip engines check for typescript-tslint-plugin package -if [[ "$(cut -d. -f1 <<< "$NODE_VERSION")" -le 6 ]]; then - nvm install 8 - nvm use 8 - yarn install --ignore-engines --ignore-scripts - # current versions of nock don't support node 6 + +# control which packages we test on each version of node +if [[ "$(cut -d. -f1 <<<"$NODE_VERSION")" -le 6 ]]; then + + # install legacy versions of packages whose current versions don't support node 6 + # ignoring engines and scripts lets us get away with having incompatible things installed for packages we're not testing cd packages/node - yarn add --dev --ignore-engines nock@10.x + yarn add --dev --ignore-engines --ignore-scripts nock@10.x + cd ../.. + cd packages/tracing + yarn add --dev --ignore-engines --ignore-scripts jsdom@11.x + cd ../.. + + # only test against @sentry/node and its dependencies - node 6 is too old for anything else to work + yarn test --scope="@sentry/core" --scope="@sentry/hub" --scope="@sentry/minimal" --scope="@sentry/node" --scope="@sentry/utils" --scope="@sentry/tracing" + +elif [[ "$(cut -d. -f1 <<<"$NODE_VERSION")" -le 8 ]]; then + + # install legacy versions of packages whose current versions don't support node 8 + # ignoring engines and scripts lets us get away with having incompatible things installed for packages we're not testing + cd packages/tracing + yarn add --dev --ignore-engines --ignore-scripts jsdom@15.x cd ../.. - # ember requires Node >= 10 to build - yarn build --ignore="@sentry/ember" --ignore="@sentry/serverless" --ignore="@sentry/gatsby" --ignore="@sentry/react" --ignore="@sentry/wasm" - nvm install 6 - nvm use 6 - # browser can be tested only on Node >= v8 because Karma is not supporting anything older - yarn test --ignore="@sentry/tracing" --ignore="@sentry/react" --ignore="@sentry/wasm" --ignore="@sentry/gatsby" --ignore="@sentry/ember" --ignore="@sentry-internal/eslint-plugin-sdk" --ignore="@sentry-internal/eslint-config-sdk" --ignore="@sentry/serverless" --ignore="@sentry/browser" --ignore="@sentry/integrations" --ignore="@sentry/utils" -elif [[ "$(cut -d. -f1 <<< "$NODE_VERSION")" -le 8 ]]; then - yarn install --ignore-engines --ignore-scripts - # ember requires Node >= 10 to build - yarn build --ignore="@sentry/ember" --ignore="@sentry/serverless" --ignore="@sentry/gatsby" --ignore="@sentry/react" --ignore="@sentry/wasm" - # serverless, tracing, ember and react work only on Node >= v10 - yarn test --ignore="@sentry/tracing" --ignore="@sentry/react" --ignore="@sentry/wasm" --ignore="@sentry/gatsby" --ignore="@sentry/ember" --ignore="@sentry-internal/eslint-plugin-sdk" --ignore="@sentry-internal/eslint-config-sdk" --ignore="@sentry/serverless" + + # ember tests happen separately, and the rest fail on node 8 for various syntax or dependency reasons + yarn test --ignore="@sentry/ember" --ignore="@sentry-internal/eslint-plugin-sdk" --ignore="@sentry/react" --ignore="@sentry/wasm" --ignore="@sentry/gatsby" --ignore="@sentry/serverless" + else - yarn install - yarn build - yarn test + yarn test --ignore="@sentry/ember" + fi