Skip to content
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

ref(nextjs): Small improvements to config tests #3938

Merged
merged 6 commits into from Aug 31, 2021

Conversation

lobsterkatie
Copy link
Member

In the process of reviewing #3922, I realized that there was some cleanup work I could do to make that PR work better. Specifically:

  • withSentryConfig had an incorrect type for one of the arguments to the userNextConfig function.
  • The mock for that same argument was missing its outer layer, and therefore wasn't really what it said it was.
  • In real life, the config property of buildContext is equal to the combination of next's default config and the user-provided config. The mock wasn't reflecting this.
  • That PR was having to compensate for the fact that the mocks for build context were static rather than dynamic, even though those test cases especially illustrate the ways in which the value of the mock needs to change depending on other values in the test. Where necessary, the build context is now calculated as part of the test.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 31, 2021

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 21.6 KB (0%)
@sentry/browser - Webpack 22.61 KB (0%)
@sentry/react - Webpack 22.64 KB (0%)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 29.03 KB (0%)

Copy link
Contributor

@kamilogorek kamilogorek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small simplification but feel free to ignore it

packages/nextjs/test/config.test.ts Outdated Show resolved Hide resolved
@lobsterkatie lobsterkatie enabled auto-merge (squash) August 31, 2021 15:09
@lobsterkatie lobsterkatie merged commit 9703ce2 into master Aug 31, 2021
@lobsterkatie lobsterkatie deleted the kmclb-nextjs-config-test-fixes-sept-21 branch August 31, 2021 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants