ref(core): Don't create spans for skipped release steps (#112)

Lms24 · web-flow · commit a0640771c182 · 2022-11-11T13:27:48.000Z
Skip creating spans if optional release pipeline steps are skipped
Adjust and add tests
diff --git a/packages/bundler-plugin-core/src/sentry/releasePipeline.ts b/packages/bundler-plugin-core/src/sentry/releasePipeline.ts
@@ -6,6 +6,7 @@
 //           - huge download
 //           - unnecessary functionality
 
+import { logger } from "@sentry/utils";
 import { InternalOptions } from "../options-mapping";
 import { BuildContext } from "../types";
 import { addSpanToTransaction } from "./telemetry";
@@ -19,6 +20,20 @@ export async function createNewRelease(options: InternalOptions, ctx: BuildConte
   span?.finish();
 }
 
+export async function cleanArtifacts(options: InternalOptions, ctx: BuildContext): Promise<void> {
+  if (!options.cleanArtifacts) {
+    logger.debug("Skipping artifact cleanup.");
+    return;
+  }
+
+  const span = addSpanToTransaction(ctx, "function.plugin.clean_artifacts");
+
+  await ctx.cli.releases.execute(["releases", "files", options.release, "delete", "--all"], true);
+
+  ctx.logger.info("Successfully cleaned previous artifacts.");
+  span?.finish();
+}
+
 export async function uploadSourceMaps(options: InternalOptions, ctx: BuildContext): Promise<void> {
   const span = addSpanToTransaction(ctx, "function.plugin.upload_sourcemaps");
   ctx.logger.info("Uploading Sourcemaps.");
@@ -31,66 +46,60 @@ export async function uploadSourceMaps(options: InternalOptions, ctx: BuildConte
   span?.finish();
 }
 
-export async function finalizeRelease(options: InternalOptions, ctx: BuildContext): Promise<void> {
-  const span = addSpanToTransaction(ctx, "function.plugin.finalize_release");
-
-  if (options.finalize) {
-    await ctx.cli.releases.finalize(options.release);
-    ctx.logger.info("Successfully finalized release.");
+export async function setCommits(options: InternalOptions, ctx: BuildContext): Promise<void> {
+  if (!options.setCommits) {
+    logger.debug("Skipping setting commits to release.");
+    return;
   }
 
-  span?.finish();
-}
-
-export async function cleanArtifacts(options: InternalOptions, ctx: BuildContext): Promise<void> {
-  const span = addSpanToTransaction(ctx, "function.plugin.clean_artifacts");
-
-  if (options.cleanArtifacts) {
-    await ctx.cli.releases.execute(["releases", "files", options.release, "delete", "--all"], true);
-    ctx.logger.info("Successfully cleaned previous artifacts.");
-  }
+  const span = addSpanToTransaction(ctx, "function.plugin.set_commits");
 
+  const { auto, repo, commit, previousCommit, ignoreMissing, ignoreEmpty } = options.setCommits;
+  await ctx.cli.releases.setCommits(options.release, {
+    commit,
+    previousCommit,
+    repo,
+    auto,
+    ignoreMissing,
+    ignoreEmpty,
+  });
+
+  ctx.logger.info("Successfully set commits.");
   span?.finish();
 }
 
-export async function setCommits(options: InternalOptions, ctx: BuildContext): Promise<void> {
-  const span = addSpanToTransaction(ctx, "function.plugin.set_commits");
-
-  if (options.setCommits) {
-    const { auto, repo, commit, previousCommit, ignoreMissing, ignoreEmpty } = options.setCommits;
+export async function finalizeRelease(options: InternalOptions, ctx: BuildContext): Promise<void> {
+  if (!options.finalize) {
+    logger.debug("Skipping release finalization.");
+    return;
+  }
 
-    await ctx.cli.releases.setCommits(options.release, {
-      commit,
-      previousCommit,
-      repo,
-      auto,
-      ignoreMissing,
-      ignoreEmpty,
-    });
+  const span = addSpanToTransaction(ctx, "function.plugin.finalize_release");
 
-    ctx.logger.info("Successfully set commits.");
-  }
+  await ctx.cli.releases.finalize(options.release);
 
+  ctx.logger.info("Successfully finalized release.");
   span?.finish();
 }
 
 export async function addDeploy(options: InternalOptions, ctx: BuildContext): Promise<void> {
-  const span = addSpanToTransaction(ctx, "function.plugin.deploy");
-
-  if (options.deploy) {
-    const { env, started, finished, time, name, url } = options.deploy;
-
-    await ctx.cli.releases.newDeploy(options.release, {
-      env,
-      started,
-      finished,
-      time,
-      name,
-      url,
-    });
-
-    ctx.logger.info("Successfully added deploy.");
+  if (!options.deploy) {
+    logger.debug("Skipping adding deploy info to release.");
+    return;
   }
 
+  const span = addSpanToTransaction(ctx, "function.plugin.deploy");
+
+  const { env, started, finished, time, name, url } = options.deploy;
+  await ctx.cli.releases.newDeploy(options.release, {
+    env,
+    started,
+    finished,
+    time,
+    name,
+    url,
+  });
+
+  ctx.logger.info("Successfully added deploy.");
   span?.finish();
 }
diff --git a/packages/bundler-plugin-core/test/releasePipeline.test.ts b/packages/bundler-plugin-core/test/releasePipeline.test.ts
@@ -1,5 +1,12 @@
 import { InternalOptions } from "../src/options-mapping";
-import { addDeploy, setCommits } from "../src/sentry/releasePipeline";
+import {
+  addDeploy,
+  cleanArtifacts,
+  createNewRelease,
+  finalizeRelease,
+  setCommits,
+  uploadSourceMaps,
+} from "../src/sentry/releasePipeline";
 import { BuildContext } from "../src/types";
 
 const mockedAddSpanToTxn = jest.fn();
@@ -11,9 +18,9 @@ jest.mock("../src/sentry/telemetry", () => {
   // eslint-disable-next-line @typescript-eslint/no-unsafe-return
   return {
     ...original,
-    addSpanToTransaction: () => {
+    addSpanToTransaction: (ctx: unknown, op: string) => {
       // eslint-disable-next-line @typescript-eslint/no-unsafe-return
-      return mockedAddSpanToTxn();
+      return mockedAddSpanToTxn(ctx, op);
     },
   };
 });
@@ -28,7 +35,11 @@ describe("Release Pipeline", () => {
 
   const mockedCLI = {
     releases: {
+      new: jest.fn(),
+      execute: jest.fn(),
+      uploadSourceMaps: jest.fn(),
       setCommits: jest.fn(),
+      finalize: jest.fn(),
       newDeploy: jest.fn(),
     },
   };
@@ -38,13 +49,71 @@ describe("Release Pipeline", () => {
 
   const ctx = { cli: mockedCLI, logger: mockedLogger };
 
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  describe("createNewRelease", () => {
+    it("makes a call to Sentry CLI's releases creation command", async () => {
+      await createNewRelease(
+        { release: "1.0.0" } as InternalOptions,
+        ctx as unknown as BuildContext
+      );
+
+      expect(mockedCLI.releases.new).toHaveBeenCalledWith("1.0.0");
+      expect(mockedAddSpanToTxn).toHaveBeenCalledWith(ctx, "function.plugin.create_release");
+      expect(mockedChildSpan.finish).toHaveBeenCalled();
+    });
+  });
+
+  describe("cleanArtifacts", () => {
+    it("doest do anything if cleanArtifacts is not true", async () => {
+      await cleanArtifacts({} as InternalOptions, ctx as unknown as BuildContext);
+
+      expect(mockedCLI.releases.execute).not.toHaveBeenCalled();
+      expect(mockedAddSpanToTxn).not.toHaveBeenCalled();
+      expect(mockedChildSpan.finish).not.toHaveBeenCalled();
+    });
+
+    it("makes a call to Sentry CLI's artifact removal command if `cleanArtifacts` is set", async () => {
+      await cleanArtifacts(
+        { release: "1.0.0", cleanArtifacts: true } as InternalOptions,
+        ctx as unknown as BuildContext
+      );
+
+      expect(mockedCLI.releases.execute).toHaveBeenCalledWith(
+        ["releases", "files", "1.0.0", "delete", "--all"],
+        true
+      );
+      expect(mockedAddSpanToTxn).toHaveBeenCalledWith(ctx, "function.plugin.clean_artifacts");
+      expect(mockedChildSpan.finish).toHaveBeenCalled();
+    });
+  });
+
+  describe("uploadSourceMaps", () => {
+    it("makes a call to Sentry CLI's sourcemaps upload command", async () => {
+      const options = {
+        release: "1.0.0",
+        include: [{ paths: ["dist"] }],
+      } as InternalOptions;
+
+      await uploadSourceMaps(options, ctx as unknown as BuildContext);
+
+      expect(mockedCLI.releases.uploadSourceMaps).toHaveBeenCalledWith("1.0.0", {
+        include: [{ paths: ["dist"] }],
+      });
+      expect(mockedAddSpanToTxn).toHaveBeenCalledWith(ctx, "function.plugin.upload_sourcemaps");
+      expect(mockedChildSpan.finish).toHaveBeenCalled();
+    });
+  });
+
   describe("setCommits", () => {
     it("doesn't do anything if `setCommits` option is not specified", async () => {
       await setCommits({} as InternalOptions, ctx as unknown as BuildContext);
 
       expect(mockedCLI.releases.setCommits).not.toHaveBeenCalled();
-      expect(mockedAddSpanToTxn).toHaveBeenCalled();
-      expect(mockedChildSpan.finish).toHaveBeenCalled();
+      expect(mockedAddSpanToTxn).not.toHaveBeenCalled();
+      expect(mockedChildSpan.finish).not.toHaveBeenCalled();
     });
 
     it("makes a call to Sentry CLI if the correct options are specified", async () => {
@@ -54,7 +123,28 @@ describe("Release Pipeline", () => {
       );
 
       expect(mockedCLI.releases.setCommits).toHaveBeenCalledWith("1.0.0", { auto: true });
-      expect(mockedAddSpanToTxn).toHaveBeenCalled();
+      expect(mockedAddSpanToTxn).toHaveBeenCalledWith(ctx, "function.plugin.set_commits");
+      expect(mockedChildSpan.finish).toHaveBeenCalled();
+    });
+  });
+
+  describe("finalizeRelease", () => {
+    it("doesn't do anything if `finalize` is not set", async () => {
+      await finalizeRelease({} as InternalOptions, ctx as unknown as BuildContext);
+
+      expect(mockedCLI.releases.finalize).not.toHaveBeenCalled();
+      expect(mockedAddSpanToTxn).not.toHaveBeenCalled();
+      expect(mockedChildSpan.finish).not.toHaveBeenCalled();
+    });
+
+    it("makes a call to Sentry CLI's release finalization command if `finalize` is true", async () => {
+      await finalizeRelease(
+        { release: "1.0.0", finalize: true } as InternalOptions,
+        ctx as unknown as BuildContext
+      );
+
+      expect(mockedCLI.releases.finalize).toHaveBeenCalledWith("1.0.0");
+      expect(mockedAddSpanToTxn).toHaveBeenCalledWith(ctx, "function.plugin.finalize_release");
       expect(mockedChildSpan.finish).toHaveBeenCalled();
     });
   });
@@ -64,8 +154,8 @@ describe("Release Pipeline", () => {
       await addDeploy({} as InternalOptions, ctx as unknown as BuildContext);
 
       expect(mockedCLI.releases.newDeploy).not.toHaveBeenCalled();
-      expect(mockedAddSpanToTxn).toHaveBeenCalled();
-      expect(mockedChildSpan.finish).toHaveBeenCalled();
+      expect(mockedAddSpanToTxn).not.toHaveBeenCalled();
+      expect(mockedChildSpan.finish).not.toHaveBeenCalled();
     });
 
     it("makes a call to Sentry CLI if the correct options are specified", async () => {
