ref(nextjs): Small changes to nextjs integration test runner (#3819)

This makes a number of small tweaks to the integration test runner in `@sentry/nextjs`, mostly changes I made to help myself as I was trying to debug failing tests in another PR, either because they made the debugging itself easier or because they sped up the overall running of the test suite (in most cases by doing things only once instead of multiple times where possible).

Included changes:

- Add a default version of `nextjs` to the integration test project’s dependencies. This makes it possible to call `yarn` (and `yarn build`) out of the context of the test runner script (specifically, when using a debugger).

- Add a VSCode debug profile for nextjs integration tests.

- Speed up initial setup by only having `yarn` install packages once per version of `next` (instead of once to install all non-`nextjs` packages, and again when adding the specific version of `next`).

- Back up `next.config.js` outside of the loops, so we’re guaranteed to restore from the original.

- As part of cleanup, nuke all of `node_modules` rather than just removing `nextjs` (we’re going to delete `node_modules` before the next run anyway, and this way `yarn` doesn’t go through yet another install process on its way out the door).

- Also as part of cleanup, remove all files added to the yarn cache as a result of this test run, in order to prevent the cache from growing arbitrarily large.

- Only run check on node version once, since it’s the same across all loops.

- Label each event/transaction/session displayed (when using `--debug` on server tests) with the name of the test it's from.

- 
Remove test for tracing 404s, since it the underlying code doesn’t actually work on its own (this is okay; we don’t capture 404s in any other framework).

- Make some formatting changes (as insisted upon by the auto-formatter).

- Add some comments and clean up logging a little bit.

.vscode/launch.json

@@ -54,6 +54,29 @@
       "internalConsoleOptions": "neverOpen", // since we're not using it, don't automatically switch to it
     },
 
+    // @sentry/nextjs - run a specific integration test file
+    // must have file in currently active tab when hitting the play button
+    {
+      "type": "node",
+      "request": "launch",
+      "cwd": "${workspaceFolder}/packages/nextjs",
+      "name": "Debug @sentry/nextjs integration tests - just open file",
+      // TODO create a build task
+      // "preLaunchTask": "yarn build",
+      "program": "${workspaceFolder}/packages/nextjs/test/integration/test/server.js",
+      "args": [
+        "--debug",
+        // remove these two lines to run all integration tests
+        "--filter",
+        "${fileBasename}"
+      ],
+      "disableOptimisticBPs": true,
+      "sourceMaps": true,
+      "skipFiles": [
+        "<node_internals>/**", "**/tslib/**"
+      ],
+    },
+
     // @sentry/node - run a specific test file in watch mode
     // must have file in currently active tab when hitting the play button
     {

packages/nextjs/test/integration/package.json

@@ -8,6 +8,7 @@
   },
   "dependencies": {
     "@sentry/nextjs": "file:../../",
+    "next": "latest",
     "react": "^17.0.1",
     "react-dom": "^17.0.1"
   },

packages/nextjs/test/integration/test/server/errorApiEndpoint.js

@@ -24,6 +24,7 @@ module.exports = async ({ url, argv }) => {
       transaction: 'GET /api/error',
     },
     argv,
+    'errorApiEndpoint',
   );
 
   await getAsync(`${url}/api/error`);

packages/nextjs/test/integration/test/server/errorServerSideProps.js

@@ -23,6 +23,7 @@ module.exports = async ({ url, argv }) => {
       },
     },
     argv,
+    'errorServerSideProps',
   );
 
   await getAsync(`${url}/withServerSideProps`);

packages/nextjs/test/integration/test/server/tracing200.js

@@ -20,6 +20,7 @@ module.exports = async ({ url, argv }) => {
       },
     },
     argv,
+    'tracing200',
   );
 
   await getAsync(`${url}/api/users`);

packages/nextjs/test/integration/test/server/tracing500.js

@@ -20,6 +20,7 @@ module.exports = async ({ url, argv }) => {
       },
     },
     argv,
+    'tracing500',
   );
 
   await getAsync(`${url}/api/broken`);

packages/nextjs/test/integration/test/server/tracingHttp.js

@@ -34,6 +34,7 @@ module.exports = async ({ url, argv }) => {
       },
     },
     argv,
+    'tracingHttp',
   );
 
   await getAsync(`${url}/api/http`);

packages/nextjs/test/integration/test/utils/server.js

@@ -21,36 +21,57 @@ const getAsync = url => {
   });
 };
 
-const interceptEventRequest = (expectedEvent, argv) => {
+const interceptEventRequest = (expectedEvent, argv, testName = '') => {
   return nock('https://dsn.ingest.sentry.io')
     .post('/api/1337/store/', body => {
-      logIf(argv.debug, 'Intercepted Event', body, argv.depth);
+      logIf(
+        argv.debug,
+        '\nIntercepted Event' + (testName.length ? ` (from test \`${testName}\`)` : ''),
+        body,
+        argv.depth,
+      );
       return objectMatches(body, expectedEvent);
     })
     .reply(200);
 };
 
-const interceptSessionRequest = (expectedItem, argv) => {
+const interceptSessionRequest = (expectedItem, argv, testName = '') => {
   return nock('https://dsn.ingest.sentry.io')
     .post('/api/1337/envelope/', body => {
       const { envelopeHeader, itemHeader, item } = parseEnvelope(body);
-      logIf(argv.debug, 'Intercepted Transaction', { envelopeHeader, itemHeader, item }, argv.depth);
+      logIf(
+        argv.debug,
+        '\nIntercepted Session' + (testName.length ? ` (from test \`${testName}\`)` : ''),
+        { envelopeHeader, itemHeader, item },
+        argv.depth,
+      );
       return itemHeader.type === 'session' && objectMatches(item, expectedItem);
     })
     .reply(200);
 };
 
-const interceptTracingRequest = (expectedItem, argv) => {
+const interceptTracingRequest = (expectedItem, argv, testName = '') => {
   return nock('https://dsn.ingest.sentry.io')
     .post('/api/1337/envelope/', body => {
       const { envelopeHeader, itemHeader, item } = parseEnvelope(body);
-      logIf(argv.debug, 'Intercepted Transaction', { envelopeHeader, itemHeader, item }, argv.depth);
+      logIf(
+        argv.debug,
+        '\nIntercepted Transaction' + (testName.length ? ` (from test \`${testName}\`)` : ''),
+        { envelopeHeader, itemHeader, item },
+        argv.depth,
+      );
       return itemHeader.type === 'transaction' && objectMatches(item, expectedItem);
     })
     .reply(200);
 };
 
+/**
+ * Recursively checks that every path/value pair in `expected` matches that in `actual` (but not vice-versa).
+ *
+ * Only works for JSONifiable data.
+ */
 const objectMatches = (actual, expected) => {
+  // each will output either '[object Object]' or '[object <ClassName>]'
   if (Object.prototype.toString.call(actual) !== Object.prototype.toString.call(expected)) {
     return false;
   }
@@ -59,11 +80,14 @@ const objectMatches = (actual, expected) => {
     const expectedValue = expected[key];
     const actualValue = actual[key];
 
+    // recurse
     if (Object.prototype.toString.call(expectedValue) === '[object Object]' || Array.isArray(expectedValue)) {
       if (!objectMatches(actualValue, expectedValue)) {
         return false;
       }
-    } else {
+    }
+    // base case
+    else {
       if (actualValue !== expectedValue) {
         return false;
       }

packages/nextjs/test/run-integration-tests.sh

@@ -2,20 +2,33 @@
 
 set -e
 
+START_TIME=$(date -R)
+
 function cleanup {
   echo "[nextjs] Cleaning up..."
-  mv next.config.js.bak next.config.js 2> /dev/null || true
-  yarn remove next > /dev/null 2>&1 || true
+  mv next.config.js.bak next.config.js 2>/dev/null || true
+  rm -rf node_modules 2>/dev/null || true
+
+  # Delete yarn's cached versions of sentry packages added during this test run, since every test run installs multiple
+  # copies of each package. Without this, the cache can balloon in size quickly if integration tests are being run
+  # multiple times in a row.
+  find $(yarn cache dir) -iname "npm-@sentry*" -newermt "$START_TIME" -mindepth 1 -maxdepth 1 -exec rm -rf {} \;
+
   echo "[nextjs] Test run complete"
 }
 
 trap cleanup EXIT
 
 cd "$(dirname "$0")/integration"
 
+NODE_VERSION=$(node -v)
+NODE_MAJOR=$(echo "$NODE_VERSION" | cut -c2- | cut -d. -f1)
+echo "Running integration tests on Node $NODE_VERSION"
+
+# make a backup of our config file so we can restore it when we're done
+mv next.config.js next.config.js.bak
+
 for NEXTJS_VERSION in 10 11; do
-  NODE_VERSION=$(node -v)
-  NODE_MAJOR=$(echo "$NODE_VERSION" | cut -c2- | cut -d. -f1)
 
   # Next 10 requires at least Node v10
   if [ "$NODE_MAJOR" -lt "10" ]; then
@@ -25,63 +38,66 @@ for NEXTJS_VERSION in 10 11; do
 
   # Next.js v11 requires at least Node v12
   if [ "$NODE_MAJOR" -lt "12" ] && [ "$NEXTJS_VERSION" -eq "11" ]; then
-    echo "[nextjs$NEXTJS_VERSION] Not compatible with Node $NODE_VERSION"
+    echo "[nextjs$NEXTJS_VERSION] Not compatible with Node $NODE_MAJOR"
     exit 0
   fi
 
-  echo "[nextjs@$NEXTJS_VERSION] Running integration tests on $NODE_VERSION"
-
   echo "[nextjs@$NEXTJS_VERSION] Preparing environment..."
-  mv next.config.js next.config.js.bak
-  rm -rf node_modules .next .env.local 2> /dev/null || true
+  rm -rf node_modules .next .env.local 2>/dev/null || true
 
   echo "[nextjs@$NEXTJS_VERSION] Installing dependencies..."
-  yarn --no-lockfile --silent > /dev/null 2>&1
-  yarn add "next@$NEXTJS_VERSION" > /dev/null 2>&1
+  # set the desired version of next long enough to run yarn, and then restore the old version (doing the restoration now
+  # rather than during overall cleanup lets us look for "latest" in both loops)
+  cp package.json package.json.bak
+  if [[ $(uname) == "Darwin" ]]; then
+    sed -i "" /"next.*latest"/s/latest/"${NEXTJS_VERSION}.x"/ package.json
+  else
+    sed -i /"next.*latest"/s/latest/"${NEXTJS_VERSION}.x"/ package.json
+  fi
+  yarn --no-lockfile --silent >/dev/null 2>&1
+  mv -f package.json.bak package.json 2>/dev/null || true
 
   for RUN_WEBPACK_5 in false true; do
     [ "$RUN_WEBPACK_5" == true ] &&
       WEBPACK_VERSION=5 ||
       WEBPACK_VERSION=4
 
+    # next 10 defaults to webpack 4 and next 11 defaults to webpack 5, but each can use either based on settings
     if [ "$NEXTJS_VERSION" -eq "10" ]; then
-      sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" < next10.config.template > next.config.js
+      sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" <next10.config.template >next.config.js
     else
-      sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" < next11.config.template > next.config.js
+      sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" <next11.config.template >next.config.js
     fi
 
     echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Building..."
     yarn build | grep "Using webpack"
 
-    # if no arguments were passed, default to outputting nothing other than success and failure messages ($* gets all
-    # passed args as a single string)
     args=$*
     if [[ ! $args ]]; then
+      # restrict each test to only output success and failure messages
       args="--silent"
     fi
 
     # we keep this updated as we run the tests, so that if it's ever non-zero, we can bail
     EXIT_CODE=0
 
-    echo "Running server tests with options: $args"
+    echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Running server tests with options: $args"
     node test/server.js $args || EXIT_CODE=$?
 
-    if [ $EXIT_CODE -eq 0 ]
-    then
+    if [ $EXIT_CODE -eq 0 ]; then
       echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Server integration tests passed"
     else
       echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Server integration tests failed"
       exit 1
     fi
 
-    echo "Running client tests with options: $args"
+    echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Running client tests with options: $args"
     node test/client.js $args || EXIT_CODE=$?
-    if [ $EXIT_CODE -eq 0 ]
-    then
+    if [ $EXIT_CODE -eq 0 ]; then
       echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Client integration tests passed"
     else
       echo "[nextjs@$NEXTJS_VERSION | webpack@$WEBPACK_VERSION] Client integration tests failed"
       exit 1
     fi
   done
-done;
+done