add and fix tests

Author: lobsterkatie

packages/nextjs/test/buildProcess/tests/nft.test.ts

@@ -11,12 +11,15 @@ it('excludes build-time SDK dependencies from nft files', () => {
 
   // These are all of the files which control the way we modify the user's app build by changing the webpack config.
   // They get mistakenly included by the nft plugin because we export `withSentryConfig` from `index.server.ts`. Because
-  // the wrappers are referenced from the code we inject at build time, they need to stay.
+  // the wrappers are referenced from the code we inject at build time, they need to stay. `withSentryConfig.ts` itself
+  // also needs to stay because nextjs loads `next.config.js` at runtime
   const sentryConfigDirEntries = dogNFTjson.files.filter(entry =>
     entry.includes('node_modules/@sentry/nextjs/build/cjs/config'),
   );
-  const withSentryConfigEntries = sentryConfigDirEntries.filter(entry => !entry.includes('config/wrappers'));
   const sentryWrapperEntries = sentryConfigDirEntries.filter(entry => entry.includes('config/wrappers'));
+  const excludedSentryConfigEntries = sentryConfigDirEntries.filter(
+    entry => !sentryWrapperEntries.includes(entry) && !entry.includes('withSentryConfig'),
+  );
 
   // Sucrase and rollup are dependencies of one of the webpack loaders we add to the config - also not needed at runtime.
   const sucraseEntries = dogNFTjson.files.filter(entry => entry.includes('node_modules/sucrase'));
@@ -25,7 +28,7 @@ it('excludes build-time SDK dependencies from nft files', () => {
   );
 
   // None of the build-time dependencies should be listed
-  expect(withSentryConfigEntries.length).toEqual(0);
+  expect(excludedSentryConfigEntries.length).toEqual(0);
   expect(sucraseEntries.length).toEqual(0);
   expect(rollupEntries.length).toEqual(0);
 

packages/nextjs/test/config/withSentryConfig.test.ts

@@ -1,6 +1,9 @@
+import * as isBuildModule from '../../src/utils/isBuild';
 import { defaultsObject, exportedNextConfig, runtimePhase, userNextConfig } from './fixtures';
 import { materializeFinalNextConfig } from './testUtils';
 
+const isBuildSpy = jest.spyOn(isBuildModule, 'isBuild').mockReturnValue(true);
+
 describe('withSentryConfig', () => {
   it('includes expected properties', () => {
     const finalConfig = materializeFinalNextConfig(exportedNextConfig);
@@ -59,4 +62,37 @@ describe('withSentryConfig', () => {
     // directly
     expect('sentry' in finalConfig).toBe(false);
   });
+
+  describe('conditional use of `constructWebpackConfigFunction`', () => {
+    // Note: In these tests, it would be nice to be able to spy on `constructWebpackConfigFunction` to see whether or
+    // not it's called, but that sets up a catch-22: If you import or require the module to spy on the function, it gets
+    // cached and the `require` call we care about (inside of `withSentryConfig`) doesn't actually run the module code.
+    // Alternatively, if we call `jest.resetModules()` after setting up the spy, then the module code *is* run a second
+    // time, but the spy belongs to the first instance of the module and therefore never registers a call. Thus we have
+    // to test whether or not the file is required instead.
+
+    it('imports from `webpack.ts` if `isBuild` returns true', () => {
+      jest.isolateModules(() => {
+        // In case this is still set from elsewhere, reset it
+        delete process.env.SENTRY_WEBPACK_MODULE_LOADED;
+
+        materializeFinalNextConfig(exportedNextConfig);
+
+        expect(process.env.SENTRY_WEBPACK_MODULE_LOADED).toEqual('true');
+      });
+    });
+
+    it("doesn't import from `webpack.ts` if `isBuild` returns false", () => {
+      jest.isolateModules(() => {
+        isBuildSpy.mockReturnValueOnce(false);
+
+        // In case this is still set from elsewhere, reset it
+        delete process.env.SENTRY_WEBPACK_MODULE_LOADED;
+
+        materializeFinalNextConfig(exportedNextConfig);
+
+        expect(process.env.SENTRY_WEBPACK_MODULE_LOADED).toBeUndefined();
+      });
+    });
+  });
 });