Repository: elastic/synthetics (ts)

Issue:
[Push command] Add error reporting for failedStaleMonitors
The Kibana api returns `failedStaleMonitors` a list of monitors intended to be deleted but unable to be due to downstream errors. This list is currently dropped on the floor. 

We should consume this list and display the feedback to the user. 



Pull request:
fix: add warnings for stale monitors
+ fix #565 
+ Creates warnings for the stale monitors if the push succeeded without any failures. If there are errors then these warnings are dropped on the floor to make room for users to fix the errors first before checking the warnings. 

Patch:
diff --git a/src/push/bundler.ts b/src/push/bundler.ts
index 83b0f3f..d54edd6 100644
--- a/src/push/bundler.ts
+++ b/src/push/bundler.ts
@@ -65,12 +65,17 @@ export class Bundler {
       const archive = archiver('zip', {
         zlib: { level: 9 },
       });
-      archive.on('error', err => reject(err));
+      archive.on('error', reject);
       output.on('close', fulfill);
       archive.pipe(output);
       for (const [path, content] of this.moduleMap.entries()) {
         const relativePath = relativeToCwd(path);
-        archive.append(content, { name: relativePath });
+        // Date is fixed to Unix epoch so the file metadata is
+        // not modified everytime when files are bundled
+        archive.append(content, {
+          name: relativePath,
+          date: new Date('1970-01-01'),
+        });
       }
       archive.finalize();
     });
diff --git a/src/push/index.ts b/src/push/index.ts
index 5c1a27c..90d6244 100644
--- a/src/push/index.ts
+++ b/src/push/index.ts
@@ -27,12 +27,13 @@ import { join } from 'path';
 import { existsSync } from 'fs';
 import { readFile, writeFile } from 'fs/promises';
 import { prompt } from 'enquirer';
-import { bold } from 'kleur/colors';
+import { bold, yellow } from 'kleur/colors';
 import {
   ok,
   formatAPIError,
   formatFailedMonitors,
   formatNotFoundError,
+  formatStaleMonitors,
 } from './request';
 import { buildMonitorSchema, createMonitors } from './monitor';
 import { ProjectSettings } from '../generator';
@@ -82,10 +83,13 @@ export async function push(monitors: Monitor[], options: PushOptions) {
           progress(chunk);
           continue;
         }
-        const { failedMonitors } = chunk;
+        const { failedMonitors, failedStaleMonitors } = chunk;
         if (failedMonitors && failedMonitors.length > 0) {
           throw formatFailedMonitors(failedMonitors);
         }
+        if (failedStaleMonitors.length > 0) {
+          write(yellow(formatStaleMonitors(failedStaleMonitors)));
+        }
       }
     }
     done('Pushed');
diff --git a/src/push/request.ts b/src/push/request.ts
index d5fa688..b7d0c08 100644
--- a/src/push/request.ts
+++ b/src/push/request.ts
@@ -80,8 +80,8 @@ export function formatAPIError(
   return outer;
 }
 
-export function formatFailedMonitors(errors: APIMonitorError[]) {
-  let outer = bold(`${symbols['failed']} Error\n`);
+function formatMonitorError(errors: APIMonitorError[]) {
+  let outer = '';
   for (const error of errors) {
     const monitorId = error.id ? `: monitor(${error.id})` : '';
     let inner = bold(`> ${error.reason}${monitorId}\n`);
@@ -91,3 +91,13 @@ export function formatFailedMonitors(errors: APIMonitorError[]) {
   }
   return outer;
 }
+
+export function formatFailedMonitors(errors: APIMonitorError[]) {
+  const heading = bold(`${symbols['failed']} Error\n`);
+  return heading + formatMonitorError(errors);
+}
+
+export function formatStaleMonitors(errors: APIMonitorError[]) {
+  const heading = bold(`${symbols['warning']} Warnings\n`);
+  return heading + formatMonitorError(errors);
+}


Test patch:
diff --git a/__tests__/push/__snapshots__/request.test.ts.snap b/__tests__/push/__snapshots__/request.test.ts.snap
index 4a014f8..0064451 100644
--- a/__tests__/push/__snapshots__/request.test.ts.snap
+++ b/__tests__/push/__snapshots__/request.test.ts.snap
@@ -18,3 +18,14 @@ exports[`Push api request format failed monitors 1`] = `
 
 "
 `;
+
+exports[`Push api request format failed stale monitors 1`] = `
+"⚠ Warnings
+   > Failed to delete monitor.: monitor(monitor-1)
+       downstream erorr
+
+   > Failed to delete monitor.: monitor(monitor-2)
+       downstream erorr
+
+"
+`;
diff --git a/__tests__/push/bundler.test.ts b/__tests__/push/bundler.test.ts
index d87a63c..81cbe7c 100644
--- a/__tests__/push/bundler.test.ts
+++ b/__tests__/push/bundler.test.ts
@@ -24,14 +24,12 @@
  */
 
 import { createReadStream } from 'fs';
-import { writeFile, unlink } from 'fs/promises';
+import { writeFile, unlink, mkdir, rm } from 'fs/promises';
 import unzipper from 'unzipper';
-import { join, dirname } from 'path';
+import { join } from 'path';
 import { generateTempPath } from '../../src/helpers';
 import { Bundler } from '../../src/push/bundler';
 
-const journeyFile = join(__dirname, '..', 'e2e', 'uptime.journey.ts');
-
 async function validateZip(content) {
   const decoded = Buffer.from(content, 'base64');
   const pathToZip = generateTempPath();
@@ -45,24 +43,45 @@ async function validateZip(content) {
     files.push(entry.path);
   }
 
-  expect(files).toEqual(['__tests__/e2e/uptime.journey.ts']);
+  expect(files).toEqual(['__tests__/push/test-bundler/bundle.journey.ts']);
   await unlink(pathToZip);
 }
 
 describe('Bundler', () => {
+  const PROJECT_DIR = join(__dirname, 'test-bundler');
+  const journeyFile = join(PROJECT_DIR, 'bundle.journey.ts');
+  const bundler = new Bundler();
+
+  beforeAll(async () => {
+    await mkdir(PROJECT_DIR, { recursive: true });
+    await writeFile(
+      journeyFile,
+      `import {journey, step, monitor} from '@elastic/synthetics';
+journey('journey 1', () => {
+  monitor.use({ id: 'duplicate id' })
+  step("step1", () => {})
+});`
+    );
+  });
+
+  afterAll(async () => {
+    await rm(PROJECT_DIR, { recursive: true });
+  });
+
   it('build journey', async () => {
-    const bundler = new Bundler();
     const content = await bundler.build(journeyFile, generateTempPath());
     await validateZip(content);
   });
 
+  it('bundle should be idempotent', async () => {
+    const content1 = await bundler.build(journeyFile, generateTempPath());
+    const content2 = await bundler.build(journeyFile, generateTempPath());
+    expect(content1).toEqual(content2);
+  });
+
   it('throw errors on incorrect path', async () => {
-    const bundler = new Bundler();
     try {
-      await bundler.build(
-        join(dirname(journeyFile), 'blah.ts'),
-        generateTempPath()
-      );
+      await bundler.build(join(PROJECT_DIR, 'blah.ts'), generateTempPath());
     } catch (e) {
       expect(e.message).toContain('ENOENT');
     }
diff --git a/__tests__/push/index.test.ts b/__tests__/push/index.test.ts
index 903ed9b..4150237 100644
--- a/__tests__/push/index.test.ts
+++ b/__tests__/push/index.test.ts
@@ -164,10 +164,11 @@ journey('duplicate name', () => monitor.use({ schedule: 20 }));`
     let server: Server;
     beforeAll(async () => {
       server = await Server.create();
+      const apiRes = { failedMonitors: [], failedStaleMonitors: [] };
       server.route(
         '/sync/s/dummy/api/synthetics/service/project/monitors',
         (req, res) => {
-          res.end(JSON.stringify({ failedMonitors: [] }));
+          res.end(JSON.stringify(apiRes));
         }
       );
       server.route(
@@ -178,7 +179,7 @@ journey('duplicate name', () => monitor.use({ schedule: 20 }));`
           // Interleaved
           res.write(JSON.stringify('chunk 2') + '\n');
           res.write(JSON.stringify('chunk 3') + '\n');
-          res.end(JSON.stringify({ failedMonitors: [] }));
+          res.end(JSON.stringify(apiRes));
         }
       );
       await fakeProjectSetup(
diff --git a/__tests__/push/request.test.ts b/__tests__/push/request.test.ts
index 39fafb8..53305c9 100644
--- a/__tests__/push/request.test.ts
+++ b/__tests__/push/request.test.ts
@@ -30,6 +30,7 @@ import {
   formatAPIError,
   formatFailedMonitors,
   formatNotFoundError,
+  formatStaleMonitors,
 } from '../../src/push/request';
 
 describe('Push api request', () => {
@@ -64,4 +65,21 @@ describe('Push api request', () => {
 
     expect(formatFailedMonitors(errors)).toMatchSnapshot();
   });
+
+  it('format failed stale monitors', () => {
+    const errors: APIMonitorError[] = [
+      {
+        id: 'monitor-1',
+        reason: 'Failed to delete monitor.',
+        details: `downstream erorr`,
+      },
+      {
+        id: 'monitor-2',
+        reason: 'Failed to delete monitor.',
+        details: `downstream erorr`,
+      },
+    ];
+
+    expect(formatStaleMonitors(errors)).toMatchSnapshot();
+  });
 });