cover more permutations with tests

Author: lobsterkatie

packages/tracing/test/hub.test.ts

@@ -33,6 +33,8 @@ describe('Hub', () => {
     it('should find a transaction which has been set on the scope if sampled = true', () => {
       const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
       const transaction = hub.startTransaction({ name: 'dogpark' });
+      transaction.sampled = true;
+
       hub.configureScope(scope => {
         scope.setSpan(transaction);
       });
@@ -42,7 +44,9 @@ describe('Hub', () => {
 
     it('should find a transaction which has been set on the scope if sampled = false', () => {
       const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
-      const transaction = hub.startTransaction({ name: 'dogpark', sampled: false });
+      const transaction = hub.startTransaction({ name: 'dogpark' });
+      transaction.sampled = false;
+
       hub.configureScope(scope => {
         scope.setSpan(transaction);
       });
@@ -59,33 +63,6 @@ describe('Hub', () => {
   });
 
   describe('transaction sampling', () => {
-    describe('tracesSampleRate and tracesSampler options', () => {
-      it("should call tracesSampler if it's defined", () => {
-        const tracesSampler = jest.fn();
-        const hub = new Hub(new BrowserClient({ tracesSampler }));
-        hub.startTransaction({ name: 'dogpark' });
-
-        expect(tracesSampler).toHaveBeenCalled();
-      });
-
-      it('should prefer tracesSampler to tracesSampleRate', () => {
-        const tracesSampler = jest.fn();
-        const hub = new Hub(new BrowserClient({ tracesSampleRate: 1, tracesSampler }));
-        hub.startTransaction({ name: 'dogpark' });
-
-        expect(tracesSampler).toHaveBeenCalled();
-      });
-
-      it('tolerates tracesSampler returning a boolean', () => {
-        const tracesSampler = jest.fn().mockReturnValue(true);
-        const hub = new Hub(new BrowserClient({ tracesSampler }));
-        const transaction = hub.startTransaction({ name: 'dogpark' });
-
-        expect(tracesSampler).toHaveBeenCalled();
-        expect(transaction.sampled).toBe(true);
-      });
-    });
-
     describe('default sample context', () => {
       it('should extract request data for default sampling context when in node', () => {
         // make sure we look like we're in node
@@ -201,25 +178,78 @@ describe('Hub', () => {
         expect(transaction.sampled).toBe(false);
       });
 
-      it('should not try to override sampling decision provided in transaction context', () => {
-        // setting tracesSampleRate to 1 means that without the override, the sampling decision should be true
+      it('should set sampled = false if tracesSampleRate is 0', () => {
+        const hub = new Hub(new BrowserClient({ tracesSampleRate: 0 }));
+        const transaction = hub.startTransaction({ name: 'dogpark' });
+
+        expect(transaction.sampled).toBe(false);
+      });
+
+      it('should set sampled = true if tracesSampleRate is 1', () => {
         const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
-        const transaction = hub.startTransaction({ name: 'dogpark', sampled: false });
+        const transaction = hub.startTransaction({ name: 'dogpark' });
 
+        expect(transaction.sampled).toBe(true);
+      });
+
+      it("should call tracesSampler if it's defined", () => {
+        const tracesSampler = jest.fn();
+        const hub = new Hub(new BrowserClient({ tracesSampler }));
+        hub.startTransaction({ name: 'dogpark' });
+
+        expect(tracesSampler).toHaveBeenCalled();
+      });
+
+      it('should set sampled = false if tracesSampler returns 0', () => {
+        const tracesSampler = jest.fn().mockReturnValue(0);
+        const hub = new Hub(new BrowserClient({ tracesSampler }));
+        const transaction = hub.startTransaction({ name: 'dogpark' });
+
+        expect(tracesSampler).toHaveBeenCalled();
         expect(transaction.sampled).toBe(false);
       });
 
-      it('should not sample transactions when tracesSampleRate is 0', () => {
-        const hub = new Hub(new BrowserClient({ tracesSampleRate: 0 }));
+      it('should set sampled = true if tracesSampler returns 1', () => {
+        const tracesSampler = jest.fn().mockReturnValue(1);
+        const hub = new Hub(new BrowserClient({ tracesSampler }));
         const transaction = hub.startTransaction({ name: 'dogpark' });
 
+        expect(tracesSampler).toHaveBeenCalled();
+        expect(transaction.sampled).toBe(true);
+      });
+
+      it('should not try to override positive sampling decision provided in transaction context', () => {
+        // so that the decision otherwise would be false
+        const tracesSampler = jest.fn().mockReturnValue(0);
+        const hub = new Hub(new BrowserClient({ tracesSampler }));
+        const transaction = hub.startTransaction({ name: 'dogpark', sampled: true });
+
+        expect(transaction.sampled).toBe(true);
+      });
+
+      it('should not try to override negative sampling decision provided in transaction context', () => {
+        // so that the decision otherwise would be true
+        const tracesSampler = jest.fn().mockReturnValue(1);
+        const hub = new Hub(new BrowserClient({ tracesSampler }));
+        const transaction = hub.startTransaction({ name: 'dogpark', sampled: false });
+
         expect(transaction.sampled).toBe(false);
       });
 
-      it('should sample transactions when tracesSampleRate is 1', () => {
-        const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
+      it('should prefer tracesSampler to tracesSampleRate', () => {
+        const tracesSampler = jest.fn();
+        const hub = new Hub(new BrowserClient({ tracesSampleRate: 1, tracesSampler }));
+        hub.startTransaction({ name: 'dogpark' });
+
+        expect(tracesSampler).toHaveBeenCalled();
+      });
+
+      it('should tolerate tracesSampler returning a boolean', () => {
+        const tracesSampler = jest.fn().mockReturnValue(true);
+        const hub = new Hub(new BrowserClient({ tracesSampler }));
         const transaction = hub.startTransaction({ name: 'dogpark' });
 
+        expect(tracesSampler).toHaveBeenCalled();
         expect(transaction.sampled).toBe(true);
       });
     });
@@ -386,11 +416,21 @@ describe('Hub', () => {
         expect(extractTraceparentData(headers['sentry-trace'])!.parentSampled).toBe(false);
       });
 
-      it('should propagate sampling decision to child transactions in fetch header', () => {
+      it('should propagate positive sampling decision to child transactions in fetch header', () => {
+        // TODO (kmclb)
+      });
+
+      it('should propagate negative sampling decision to child transactions in fetch header', () => {
         // TODO (kmclb)
       });
 
-      it("should inherit parent's sampling decision when creating a new transaction if tracesSampler is undefined", () => {
+      it("should inherit parent's positive sampling decision if tracesSampler is undefined", () => {
+        // This is untestable, because there's no way to guarantee that a non-inherited decision is false (thus proving
+        // the parent's positive decision takes precedence) without using tracesSampler, since setting tracesSampleRate
+        // to 0 here will disable tracing all together. See below for the opposite test, though.
+      });
+
+      it("should inherit parent's negative sampling decision if tracesSampler is undefined", () => {
         // tracesSampleRate = 1 means every transaction should end up with sampled = true, so make parent's decision the
         // opposite to prove that inheritance takes precedence over tracesSampleRate
         const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
@@ -405,7 +445,7 @@ describe('Hub', () => {
         expect(transaction.sampled).toBe(parentSamplingDecsion);
       });
 
-      it("should ignore parent's sampling decision when tracesSampler is defined", () => {
+      it("should ignore parent's positive sampling decision when tracesSampler is defined", () => {
         // this tracesSampler causes every transaction to end up with sampled = true, so make parent's decision the
         // opposite to prove that tracesSampler takes precedence over inheritance
         const tracesSampler = () => true;
@@ -421,6 +461,23 @@ describe('Hub', () => {
 
         expect(transaction.sampled).not.toBe(parentSamplingDecsion);
       });
+
+      it("should ignore parent's negative sampling decision when tracesSampler is defined", () => {
+        // this tracesSampler causes every transaction to end up with sampled = false, so make parent's decision the
+        // opposite to prove that tracesSampler takes precedence over inheritance
+        const tracesSampler = () => false;
+        const parentSamplingDecsion = true;
+
+        const hub = new Hub(new BrowserClient({ tracesSampler }));
+
+        const transaction = hub.startTransaction({
+          name: 'dogpark',
+          parentSpanId: '12312012',
+          parentSampled: parentSamplingDecsion,
+        });
+
+        expect(transaction.sampled).not.toBe(parentSamplingDecsion);
+      });
     });
   });
 });