fix cases that were not actually working

Author: mydea

dev-packages/node-integration-tests/README.md

@@ -14,20 +14,11 @@ suites/
             |---- scenario_2.ts [optional extra test scenario]
             |---- server_with_mongo.ts [optional custom server]
             |---- server_with_postgres.ts [optional custom server]
-utils/
-|---- defaults/
-      |---- server.ts [default Express server configuration]
 ```
 
 The tests are grouped by their scopes, such as `public-api` or `tracing`. In every group of tests, there are multiple
 folders containing test scenarios and assertions.
 
-Tests run on Express servers (a server instance per test). By default, a simple server template inside
-`utils/defaults/server.ts` is used. Every server instance runs on a different port.
-
-A custom server configuration can be used, supplying a script that exports a valid express server instance as default.
-`runServer` utility function accepts an optional `serverPath` argument for this purpose.
-
 `scenario.ts` contains the initialization logic and the test subject. By default, `{TEST_DIR}/scenario.ts` is used, but
 `runServer` also accepts an optional `scenarioPath` argument for non-standard usage.
 

dev-packages/node-integration-tests/suites/express/handle-error-scope-data-loss/test.ts

@@ -14,7 +14,7 @@ afterAll(() => {
  * This test nevertheless covers the behavior so that we're aware.
  */
 test('withScope scope is NOT applied to thrown error caught by global handler', done => {
-  const runner = createRunner(__dirname, 'server.ts')
+  createRunner(__dirname, 'server.ts')
     .expect({
       event: {
         exception: {
@@ -42,16 +42,15 @@ test('withScope scope is NOT applied to thrown error caught by global handler',
         tags: expect.not.objectContaining({ local: expect.anything() }),
       },
     })
-    .start(done);
-
-  expect(() => runner.makeRequest('get', '/test/withScope')).rejects.toThrow();
+    .start(done)
+    .makeRequest('get', '/test/withScope', { expectError: true });
 });
 
 /**
  * This test shows that the isolation scope set tags are applied correctly to the error.
  */
 test('isolation scope is applied to thrown error caught by global handler', done => {
-  const runner = createRunner(__dirname, 'server.ts')
+  createRunner(__dirname, 'server.ts')
     .expect({
       event: {
         exception: {
@@ -81,7 +80,6 @@ test('isolation scope is applied to thrown error caught by global handler', done
         },
       },
     })
-    .start(done);
-
-  expect(() => runner.makeRequest('get', '/test/isolationScope')).rejects.toThrow();
+    .start(done)
+    .makeRequest('get', '/test/isolationScope', { expectError: true });
 });

dev-packages/node-integration-tests/suites/express/handle-error-tracesSampleRate-0/test.ts

@@ -5,7 +5,7 @@ afterAll(() => {
 });
 
 test('should capture and send Express controller error with txn name if tracesSampleRate is 0', done => {
-  const runner = createRunner(__dirname, 'server.ts')
+  createRunner(__dirname, 'server.ts')
     .ignore('transaction')
     .expect({
       event: {
@@ -33,7 +33,6 @@ test('should capture and send Express controller error with txn name if tracesSa
         transaction: 'GET /test/express/:id',
       },
     })
-    .start(done);
-
-  expect(() => runner.makeRequest('get', '/test/express/123')).rejects.toThrow();
+    .start(done)
+    .makeRequest('get', '/test/express/123', { expectError: true });
 });

dev-packages/node-integration-tests/suites/express/handle-error-tracesSampleRate-unset/test.ts

@@ -5,7 +5,7 @@ afterAll(() => {
 });
 
 test('should capture and send Express controller error if tracesSampleRate is not set.', done => {
-  const runner = createRunner(__dirname, 'server.ts')
+  createRunner(__dirname, 'server.ts')
     .ignore('transaction')
     .expect({
       event: {
@@ -32,7 +32,6 @@ test('should capture and send Express controller error if tracesSampleRate is no
         },
       },
     })
-    .start(done);
-
-  expect(() => runner.makeRequest('get', '/test/express/123')).rejects.toThrow();
+    .start(done)
+    .makeRequest('get', '/test/express/123', { expectError: true });
 });

dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts

@@ -9,7 +9,9 @@ test('Should overwrite baggage if the incoming request already has Sentry baggag
   const runner = createRunner(__dirname, '..', 'server.ts').start();
 
   const response = await runner.makeRequest<TestAPIResponse>('get', '/test/express', {
-    baggage: 'sentry-release=2.0.0,sentry-environment=myEnv',
+    headers: {
+      baggage: 'sentry-release=2.0.0,sentry-environment=myEnv',
+    },
   });
 
   expect(response).toBeDefined();
@@ -25,8 +27,10 @@ test('Should propagate sentry trace baggage data from an incoming to an outgoing
   const runner = createRunner(__dirname, '..', 'server.ts').start();
 
   const response = await runner.makeRequest<TestAPIResponse>('get', '/test/express', {
-    'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
-    baggage: 'sentry-release=2.0.0,sentry-environment=myEnv,dogs=great',
+    headers: {
+      'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
+      baggage: 'sentry-release=2.0.0,sentry-environment=myEnv,dogs=great',
+    },
   });
 
   expect(response).toBeDefined();
@@ -42,8 +46,10 @@ test('Should not propagate baggage data from an incoming to an outgoing request
   const runner = createRunner(__dirname, '..', 'server.ts').start();
 
   const response = await runner.makeRequest<TestAPIResponse>('get', '/test/express', {
-    'sentry-trace': '',
-    baggage: 'sentry-release=2.0.0,sentry-environment=myEnv,dogs=great',
+    headers: {
+      'sentry-trace': '',
+      baggage: 'sentry-release=2.0.0,sentry-environment=myEnv,dogs=great',
+    },
   });
 
   expect(response).toBeDefined();
@@ -59,7 +65,9 @@ test('Should not propagate baggage if sentry-trace header is present in incoming
   const runner = createRunner(__dirname, '..', 'server.ts').start();
 
   const response = await runner.makeRequest<TestAPIResponse>('get', '/test/express', {
-    'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
+    headers: {
+      'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
+    },
   });
 
   expect(response).toBeDefined();
@@ -74,8 +82,10 @@ test('Should not propagate baggage and ignore original 3rd party baggage entries
   const runner = createRunner(__dirname, '..', 'server.ts').start();
 
   const response = await runner.makeRequest<TestAPIResponse>('get', '/test/express', {
-    'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
-    baggage: 'foo=bar',
+    headers: {
+      'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
+      baggage: 'foo=bar',
+    },
   });
 
   expect(response).toBeDefined();
@@ -107,7 +117,9 @@ test('Should populate Sentry and ignore 3rd party content if sentry-trace header
   const runner = createRunner(__dirname, '..', 'server.ts').start();
 
   const response = await runner.makeRequest<TestAPIResponse>('get', '/test/express', {
-    baggage: 'foo=bar,bar=baz',
+    headers: {
+      baggage: 'foo=bar,bar=baz',
+    },
   });
 
   expect(response).toBeDefined();

dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts

@@ -9,8 +9,10 @@ test('should ignore sentry-values in `baggage` header of a third party vendor an
   const runner = createRunner(__dirname, 'server.ts').start();
 
   const response = await runner.makeRequest<TestAPIResponse>('get', '/test/express', {
-    'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
-    baggage: 'sentry-release=2.1.0,sentry-environment=myEnv',
+    headers: {
+      'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
+      baggage: 'sentry-release=2.1.0,sentry-environment=myEnv',
+    },
   });
 
   expect(response).toBeDefined();

dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors/test.ts

@@ -9,8 +9,10 @@ test('should merge `baggage` header of a third party vendor with the Sentry DSC
   const runner = createRunner(__dirname, 'server.ts').start();
 
   const response = await runner.makeRequest<TestAPIResponse>('get', '/test/express', {
-    'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
-    baggage: 'sentry-release=2.0.0,sentry-environment=myEnv',
+    headers: {
+      'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
+      baggage: 'sentry-release=2.0.0,sentry-environment=myEnv',
+    },
   });
 
   expect(response).toBeDefined();

dev-packages/node-integration-tests/suites/express/sentry-trace/trace-header-assign/test.ts

@@ -10,7 +10,9 @@ test('Should assign `sentry-trace` header which sets parent trace id of an outgo
   const runner = createRunner(__dirname, 'server.ts').start();
 
   const response = await runner.makeRequest<TestAPIResponse>('get', '/test/express', {
-    'sentry-trace': '12312012123120121231201212312012-1121201211212012-0',
+    headers: {
+      'sentry-trace': '12312012123120121231201212312012-1121201211212012-0',
+    },
   });
 
   expect(response).toBeDefined();

dev-packages/node-integration-tests/suites/express/setupExpressErrorHandler/test.ts

@@ -22,9 +22,9 @@ describe('express setupExpressErrorHandler', () => {
         .start(done);
 
       // this error is filtered & ignored
-      expect(() => runner.makeRequest('get', '/test1')).rejects.toThrow();
+      runner.makeRequest('get', '/test1', { expectError: true });
       // this error is actually captured
-      expect(() => runner.makeRequest('get', '/test2')).rejects.toThrow();
+      runner.makeRequest('get', '/test2', { expectError: true });
     });
   });
 });

dev-packages/node-integration-tests/suites/public-api/startSpan/with-nested-spans/test.ts

@@ -1,5 +1,6 @@
 import type { SpanJSON } from '@sentry/types';
-import { assertSentryTransaction, cleanupChildProcesses, createRunner } from '../../../../utils/runner';
+import { assertSentryTransaction } from '../../../../utils/assertions';
+import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';
 
 afterAll(() => {
   cleanupChildProcesses();

dev-packages/node-integration-tests/suites/sessions/crashed-session-aggregate/test.ts

@@ -4,16 +4,10 @@ afterEach(() => {
   cleanupChildProcesses();
 });
 
-test('should aggregate successful and crashed sessions', async () => {
-  let _done: undefined | (() => void);
-  const promise = new Promise<void>(resolve => {
-    _done = resolve;
-  });
-
-  const runner = createRunner(__dirname, 'server.ts')
+test('should aggregate successful and crashed sessions', done => {
+  const runner = createRunner(__dirname, '..', 'server.ts')
     .ignore('transaction', 'event')
     .unignore('sessions')
-    .expectError()
     .expect({
       sessions: {
         aggregates: [
@@ -25,11 +19,9 @@ test('should aggregate successful and crashed sessions', async () => {
         ],
       },
     })
-    .start(_done);
-
-  runner.makeRequest('get', '/success');
-  runner.makeRequest('get', '/error_unhandled');
-  runner.makeRequest('get', '/success_next');
+    .start(done);
 
-  await promise;
+  runner.makeRequest('get', '/test/success');
+  runner.makeRequest('get', '/test/error_unhandled', { expectError: true });
+  runner.makeRequest('get', '/test/success_next');
 });

dev-packages/node-integration-tests/suites/sessions/errored-session-aggregate/test.ts

@@ -4,16 +4,10 @@ afterEach(() => {
   cleanupChildProcesses();
 });
 
-test('should aggregate successful, crashed and erroneous sessions', async () => {
-  let _done: undefined | (() => void);
-  const promise = new Promise<void>(resolve => {
-    _done = resolve;
-  });
-
-  const runner = createRunner(__dirname, 'server.ts')
+test('should aggregate successful, crashed and erroneous sessions', done => {
+  const runner = createRunner(__dirname, '..', 'server.ts')
     .ignore('transaction', 'event')
     .unignore('sessions')
-    .expectError()
     .expect({
       sessions: {
         aggregates: [
@@ -26,11 +20,9 @@ test('should aggregate successful, crashed and erroneous sessions', async () =>
         ],
       },
     })
-    .start(_done);
-
-  runner.makeRequest('get', '/success');
-  runner.makeRequest('get', '/error_handled');
-  runner.makeRequest('get', '/error_unhandled');
+    .start(done);
 
-  await promise;
+  runner.makeRequest('get', '/test/success');
+  runner.makeRequest('get', '/test/error_handled');
+  runner.makeRequest('get', '/test/error_unhandled', { expectError: true });
 });

dev-packages/node-integration-tests/suites/sessions/exited-session-aggregate/test.ts

@@ -4,16 +4,10 @@ afterEach(() => {
   cleanupChildProcesses();
 });
 
-test('should aggregate successful sessions', async () => {
-  let _done: undefined | (() => void);
-  const promise = new Promise<void>(resolve => {
-    _done = resolve;
-  });
-
-  const runner = createRunner(__dirname, 'server.ts')
+test('should aggregate successful sessions', done => {
+  const runner = createRunner(__dirname, '..', 'server.ts')
     .ignore('transaction', 'event')
     .unignore('sessions')
-    .expectError()
     .expect({
       sessions: {
         aggregates: [
@@ -24,11 +18,9 @@ test('should aggregate successful sessions', async () => {
         ],
       },
     })
-    .start(_done);
-
-  runner.makeRequest('get', '/success');
-  runner.makeRequest('get', '/success_next');
-  runner.makeRequest('get', '/success_slow');
+    .start(done);
 
-  await promise;
+  runner.makeRequest('get', '/test/success');
+  runner.makeRequest('get', '/test/success_next');
+  runner.makeRequest('get', '/test/success_slow');
 });

dev-packages/node-integration-tests/suites/sessions/server.ts

@@ -1,26 +1,24 @@
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
 import type { SessionFlusher } from '@sentry/core';
 import * as Sentry from '@sentry/node';
 
 Sentry.init({
   dsn: 'https://public@dsn.ingest.sentry.io/1337',
   release: '1.0',
+  transport: loggingTransport,
 });
 
+import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
 import express from 'express';
 
 const app = express();
 
-// ### Taken from manual tests ###
-// Hack that resets the 60s default flush interval, and replaces it with just a one second interval
 const flusher = (Sentry.getClient() as Sentry.NodeClient)['_sessionFlusher'] as SessionFlusher;
 
-let flusherIntervalId = flusher && flusher['_intervalId'];
-
-clearInterval(flusherIntervalId);
-
-flusherIntervalId = flusher['_intervalId'] = setInterval(() => flusher?.flush(), 2000);
-
-setTimeout(() => clearInterval(flusherIntervalId), 4000);
+// Flush after 2 seconds (to avoid waiting for the default 60s)
+setTimeout(() => {
+  flusher?.flush();
+}, 2000);
 
 app.get('/test/success', (_req, res) => {
   res.send('Success!');
@@ -52,4 +50,4 @@ app.get('/test/error_handled', (_req, res) => {
 
 Sentry.setupExpressErrorHandler(app);
 
-export default app;
+startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/tracing/connect/test.ts

@@ -45,7 +45,6 @@ describe('connect auto-instrumentation', () => {
   test('CJS - should capture errors in `connect` middleware.', done => {
     createRunner(__dirname, 'scenario.js')
       .ignore('transaction')
-      .expectError()
       .expect({ event: EXPECTED_EVENT })
       .start(done)
       .makeRequest('get', '/error');
@@ -55,7 +54,6 @@ describe('connect auto-instrumentation', () => {
     createRunner(__dirname, 'scenario.js')
       .ignore('event')
       .expect({ transaction: { transaction: 'GET /error' } })
-      .expectError()
       .start(done)
       .makeRequest('get', '/error');
   });