Add better comments around artifact upload tests

Author: aeisenberg

lib/debug-artifacts.test.js

@@ -20,7 +20,15 @@ test("sanitizeArtifactName", (t) => {
   );
 });
 
-test("uploadDebugArtifacts when artifacts empty", async (t) => {
+// These next tests check the correctness of the logic to determine whether or not
+// artifacts are uploaded in debug mode. Since it's not easy to mock the actual
+// call to upload an artifact, we just check that we get an "upload-failed" result,
+// instead of actually uploading the artifact.
+//
+// For tests where we expect artifact upload to be blocked, we check for a different
+// response from the function.
+
+test("uploadDebugArtifacts when artifacts empty should emit 'no-artifacts-to-upload'", async (t) => {
   // Test that no error is thrown if artifacts list is empty.
   const logger = getActionsLogger();
   await t.notThrowsAsync(async () => {
@@ -40,7 +48,7 @@ test("uploadDebugArtifacts when artifacts empty", async (t) => {
   });
 });
 
-test("uploadDebugArtifacts when no codeql version is used", async (t) => {
+test("uploadDebugArtifacts when no codeql version is used should invoke artifact upload", async (t) => {
   // Test that the artifact is uploaded.
   const logger = getActionsLogger();
   await t.notThrowsAsync(async () => {
@@ -54,13 +62,14 @@ test("uploadDebugArtifacts when no codeql version is used", async (t) => {
     );
     t.is(
       uploaded,
+      // The failure is expected since we don't want to actually upload any artifacts in unit tests.
       "upload-failed",
       "Expect failure to upload artifacts since root dir does not exist",
     );
   });
 });
 
-test("uploadDebugArtifacts when new codeql version is used", async (t) => {
+test("uploadDebugArtifacts when new codeql version is used should invoke artifact upload", async (t) => {
   // Test that the artifact is uploaded.
   const logger = getActionsLogger();
   await t.notThrowsAsync(async () => {
@@ -74,13 +83,14 @@ test("uploadDebugArtifacts when new codeql version is used", async (t) => {
     );
     t.is(
       uploaded,
+      // The failure is expected since we don't want to actually upload any artifacts in unit tests.
       "upload-failed",
       "Expect failure to upload artifacts since root dir does not exist",
     );
   });
 });
 
-test("uploadDebugArtifacts when old codeql is used", async (t) => {
+test("uploadDebugArtifacts when old codeql is used should avoid trying to upload artifacts", async (t) => {
   // Test that the artifact is not uploaded.
   const logger = getActionsLogger();
   await t.notThrowsAsync(async () => {