Browse Source

feat(test): Fix console.error getting swallowed up (#38833)

Oops I seemed to have broken this awhile back so we have been swallowing
`console.error()` calls that pass in `Error` objects (i.e. React
warnings call it with strings). I've removed the custom code and added
https://github.com/ValentinH/jest-fail-on-console

Thanks to @eliashussary for discovering this.

Requires https://github.com/getsentry/getsentry/pull/8303
Billy Vong 2 years ago
parent
commit
9eb677e682

+ 4 - 3
config/tsconfig.build.json

@@ -4,9 +4,10 @@
     "allowJs": true
   },
   "exclude": [
-    "tests/js",
-    "fixtures/js-stubs",
+    "../tests/js",
+    "../fixtures/js-stubs",
     "../static/app/**/*.spec.*",
-    "../static/app/**/*.benchmark.ts"
+    "../static/app/**/*.benchmark.ts",
+    "../static/app/views/settings/project/server-side-sampling/testUtils.tsx"
   ]
 }

+ 1 - 2
jest.config.ts

@@ -166,7 +166,6 @@ const config: Config.InitialOptions = {
   },
   setupFiles: [
     '<rootDir>/static/app/utils/silence-react-unsafe-warnings.ts',
-    '<rootDir>/tests/js/throw-on-react-error.js',
     'jest-canvas-mock',
   ],
   setupFilesAfterEnv: [
@@ -218,7 +217,7 @@ const config: Config.InitialOptions = {
       init: {
         // jest project under Sentry organization (dev productivity team)
         dsn: 'https://3fe1dce93e3a4267979ebad67f3de327@sentry.io/4857230',
-        environment: !!CI ? 'ci' : 'local',
+        environment: CI ? 'ci' : 'local',
         tracesSampleRate: 1.0,
       },
       transactionOptions: {

+ 1 - 0
package.json

@@ -190,6 +190,7 @@
     "jest": "28.1.2",
     "jest-canvas-mock": "^2.4.0",
     "jest-environment-jsdom": "^28.1.2",
+    "jest-fail-on-console": "^3.0.1",
     "jest-fetch-mock": "^3.0.3",
     "jest-junit": "13.2.0",
     "jest-sentry-environment": "3.0.0",

+ 20 - 6
static/app/components/charts/eventsChart.spec.jsx

@@ -1,3 +1,5 @@
+import {act} from 'react-dom/test-utils';
+
 import {chart, doZoom, mockZoomRange} from 'sentry-test/charts';
 import {mountWithTheme} from 'sentry-test/enzyme';
 import {initializeOrg} from 'sentry-test/initializeOrg';
@@ -26,10 +28,8 @@ describe('EventsChart', function () {
   let render;
   let wrapper;
 
-  beforeEach(function () {
-    globalSelection.updateDateTime.mockClear();
-    mockZoomRange(1543449600000, 1543708800000);
-    wrapper = mountWithTheme(
+  function renderChart(props) {
+    return mountWithTheme(
       <EventsChart
         api={new MockApiClient()}
         location={{query: {}}}
@@ -41,9 +41,16 @@ describe('EventsChart', function () {
         end={null}
         utc={false}
         router={router}
+        {...props}
       />,
       routerContext
     );
+  }
+
+  beforeEach(function () {
+    globalSelection.updateDateTime.mockClear();
+    mockZoomRange(1543449600000, 1543708800000);
+    wrapper = renderChart();
 
     // XXX: Note we spy on this AFTER it has already rendered once!
     render = jest.spyOn(wrapper.find('ChartZoom').instance(), 'render');
@@ -156,8 +163,15 @@ describe('EventsChart', function () {
     expect(chartZoomInstance.history).toHaveLength(0);
   });
 
-  it('renders with World Map when given WorldMapChart chartComponent', function () {
-    wrapper.setProps({chartComponent: WorldMapChart, yAxis: ['count()']});
+  it('renders with World Map when given WorldMapChart chartComponent', async function () {
+    MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/releases/stats/',
+      body: [],
+    });
+    await act(async () => {
+      wrapper = renderChart({chartComponent: WorldMapChart, yAxis: ['count()']});
+      await tick();
+    });
     expect(wrapper.find('WorldMapChart').length).toEqual(1);
   });
 });

+ 2 - 0
static/app/components/idBadge/index.spec.tsx

@@ -28,6 +28,8 @@ describe('IdBadge', function () {
   });
 
   it('throws when no valid properties are passed', function () {
+    // Error is expected, do not fail when calling console.error
+    jest.spyOn(console, 'error').mockImplementation();
     expect(() => render(<IdBadge />)).toThrow();
   });
 });

+ 8 - 1
static/app/components/modals/widgetViewerModal.spec.tsx

@@ -1032,7 +1032,14 @@ describe('Modals -> WidgetViewerModal', function () {
         it('always queries geo.country_code in the table chart', async function () {
           const eventsv2Mock = mockEventsv2();
           mockEventsGeo();
-          await renderModal({initialData, widget: mockWidget});
+          // Getting the following console.error from worldMapChart
+          // "Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function."
+          // Ignoring it
+          jest.spyOn(console, 'error').mockImplementation();
+          await act(async () => {
+            renderModal({initialData, widget: mockWidget});
+            await tick();
+          });
           expect(eventsv2Mock).toHaveBeenCalledWith(
             '/organizations/org-slug/eventsv2/',
             expect.objectContaining({

+ 5 - 3
static/app/components/version.spec.tsx

@@ -1,4 +1,4 @@
-import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
+import {act, render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
 
 import Version from 'sentry/components/version';
 
@@ -42,8 +42,10 @@ describe('Version', () => {
     expect(screen.queryByText(VERSION)).not.toBeInTheDocument();
 
     // Activate tooltip
-    userEvent.hover(screen.getByText('1.0.0 (20200101)'));
-    jest.advanceTimersByTime(50);
+    act(() => {
+      userEvent.hover(screen.getByText('1.0.0 (20200101)'));
+      jest.advanceTimersByTime(50);
+    });
 
     expect(screen.getByText(VERSION)).toBeInTheDocument();
   });

+ 0 - 1
static/app/locale.tsx

@@ -72,7 +72,6 @@ function getClient(): Jed | null {
     // If this happens, it could mean that an import was added/changed where
     // locale initialization does not happen soon enough.
     const warning = new Error('Locale not set, defaulting to English');
-    console.error(warning); // eslint-disable-line no-console
     Sentry.captureException(warning);
     return setLocale(DEFAULT_LOCALE_DATA);
   }

+ 5 - 7
static/app/utils/useMetricsContext.spec.tsx

@@ -92,12 +92,10 @@ describe('useMetricsContext', function () {
   });
 
   it('throw when provider is not set', function () {
-    try {
-      render(<TestComponent other="value" />);
-    } catch (error) {
-      expect(error.message).toEqual(
-        'useMetricsContext was called outside of MetricsProvider'
-      );
-    }
+    // Error is expected, do not fail when calling console.error
+    jest.spyOn(console, 'error').mockImplementation();
+    expect(() => render(<TestComponent other="value" />)).toThrow(
+      /useMetricsContext was called outside of MetricsProvider/
+    );
   });
 });

+ 7 - 7
static/app/utils/useRoutes.spec.tsx

@@ -35,10 +35,12 @@ describe('useRoutes', () => {
   });
 
   it('throws error when called outside of routes provider', function () {
-    try {
-      const memoryHistory = createMemoryHistory();
-      memoryHistory.push('/');
+    // Error is expected, do not fail when calling console.error
+    jest.spyOn(console, 'error').mockImplementation();
+    const memoryHistory = createMemoryHistory();
+    memoryHistory.push('/');
 
+    expect(() =>
       render(
         <Router history={memoryHistory}>
           <Route
@@ -49,9 +51,7 @@ describe('useRoutes', () => {
             }}
           />
         </Router>
-      );
-    } catch (error) {
-      expect(error.message).toBe('useRouteContext called outside of routes provider');
-    }
+      )
+    ).toThrow(/useRouteContext called outside of routes provider/);
   });
 });

Some files were not shown because too many files changed in this diff