From 52faf5649947ba7681be73fdc7b020a85627535f Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 23 Nov 2022 00:53:04 -0800 Subject: [PATCH] add and fix tests --- .../test/buildProcess/tests/nft.test.ts | 9 +++-- .../test/config/withSentryConfig.test.ts | 36 +++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/packages/nextjs/test/buildProcess/tests/nft.test.ts b/packages/nextjs/test/buildProcess/tests/nft.test.ts index 0915489a88b1..fb208a9cae0c 100644 --- a/packages/nextjs/test/buildProcess/tests/nft.test.ts +++ b/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); diff --git a/packages/nextjs/test/config/withSentryConfig.test.ts b/packages/nextjs/test/config/withSentryConfig.test.ts index 7ae6589c8231..e726df33340c 100644 --- a/packages/nextjs/test/config/withSentryConfig.test.ts +++ b/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(); + }); + }); + }); });