Update development disable command to pull service by default (#110)

* Currently when service is disabled in development mode if a user does not delete it or pull it manually, the image will remain to be the image built by development mode potentially.
* Pulling the image after disabling will mean that the service is reloaded as the one held in ECR
* Added the `-P` to the disable command to maintain current behaviour if desired
* Reorganied `disable.spec.ts` mocks to use automatic mocks and not manual mock
* Add a method which can be overidden in children of AbstractStateModificationCommand so that actions can occur after the hook has successfully run

README.md

@@ -232,11 +232,14 @@ Removes a service from development mode
 
 ```
 USAGE
-  $ chs-dev development disable SERVICES...
+  $ chs-dev development disable SERVICES... [-P]
 
 ARGUMENTS
   SERVICES...  names of services to be removed to development mode
 
+FLAGS
+  -P, --noPull  Does not perform a docker compose pull to reset the service to what is stored in ECR
+
 DESCRIPTION
   Removes a service from development mode
 ```

src/commands/AbstractStateModificationCommand.ts

@@ -58,7 +58,9 @@ export default abstract class AbstractStateModificationCommand extends Command {
         }
 
         if (runHook) {
-            this.config.runHook("generate-runnable-docker-compose", {});
+            await this.config.runHook("generate-runnable-docker-compose", {});
+
+            await this.handlePostHookCall(argv as string[]);
         }
     }
 
@@ -69,6 +71,8 @@ export default abstract class AbstractStateModificationCommand extends Command {
         return this.parse(AbstractStateModificationCommand);
     }
 
+    protected async handlePostHookCall (_: string[]): Promise<void> {}
+
     private get properStateModificationObjectType (): string {
         return `${this.stateModificationObjectType.substring(0, 1).toUpperCase()}${this.stateModificationObjectType.substring(1)}`;
     }

src/commands/development/disable.ts

@@ -1,6 +1,7 @@
-import { Args, Config } from "@oclif/core";
+import { Args, Config, Flags, ux } from "@oclif/core";
 import AbstractStateModificationCommand from "../AbstractStateModificationCommand.js";
 import { serviceValidator } from "../../helpers/validator.js";
+import { DockerCompose } from "../../run/docker-compose.js";
 
 export default class Disable extends AbstractStateModificationCommand {
 
@@ -14,16 +15,52 @@ export default class Disable extends AbstractStateModificationCommand {
         })
     };
 
+    static flags = {
+        noPull: Flags.boolean({
+            name: "no-pull",
+            required: false,
+            default: false,
+            char: "P",
+            aliases: ["noPull"],
+            description: "Does not perform a docker compose pull to reset the service to what is stored in ECR"
+        })
+    };
+
+    private readonly dockerCompose: DockerCompose;
+
     constructor (argv: string[], config: Config) {
         super(argv, config, "service");
 
         this.argumentValidationPredicate = serviceValidator(this.inventory, this.error, true);
         this.validArgumentHandler = this.handleValidService;
+        this.dockerCompose = new DockerCompose(
+            this.chsDevConfig, {
+                log: this.log
+            }
+        );
     }
 
     private async handleValidService (serviceName: string): Promise<void> {
         this.stateManager.excludeServiceFromLiveUpdate(serviceName);
-        this.log(`Service "${serviceName}" is disabled`);
     }
 
+    protected async handlePostHookCall (serviceNames: string[]): Promise<void> {
+
+        const noPull = this.flagValues?.noPull || false;
+
+        if (!noPull) {
+            for (const serviceName of serviceNames) {
+                ux.action.start(`pulling service container: ${serviceName}`);
+                await this.dockerCompose.pull(serviceName);
+                ux.action.stop();
+
+                this.log(`Service "${serviceName}" is disabled`);
+            }
+
+        }
+    }
+
+    protected parseArgumentsAndFlags (): Promise<{ argv: unknown[]; flags: Record<string, any>; }> {
+        return this.parse(Disable);
+    }
 }

src/run/docker-compose.ts

@@ -8,6 +8,7 @@ import Config from "../model/Config.js";
 import LogEverythingLogHandler from "./logs/LogEverythingLogHandler.js";
 import { spawn } from "../helpers/spawn-promise.js";
 import { runStatusColouriser, stopStatusColouriser } from "../helpers/colouriser.js";
+import LogNothingLogHandler from "./logs/LogNothingLogHandler.js";
 
 interface Logger {
     log: (msg: string) => void;
@@ -117,6 +118,17 @@ export class DockerCompose {
         signal);
     }
 
+    pull (serviceName: string, abortSignal?: AbortSignal): Promise<void> {
+        return this.runDockerCompose(
+            [
+                "pull",
+                serviceName
+            ],
+            new LogNothingLogHandler(this.logFile, this.logger),
+            abortSignal
+        );
+    }
+
     private createStatusMatchLogHandler (pattern: RegExp, colouriser?: (status: string) => string) {
         return new PatternMatchingConsoleLogHandler(
             pattern, this.logFile, this.logger, colouriser

src/run/logs/LogNothingLogHandler.ts

@@ -0,0 +1,8 @@
+import { AbstractLogHandler, Logger, LogHandler } from "./logs-handler.js";
+
+export class LogNothingLogHandler extends AbstractLogHandler {
+    protected logToConsole (_: string[]): void { }
+
+}
+
+export default LogNothingLogHandler;

test/commands/development/disable.spec.ts

@@ -1,32 +1,34 @@
 import { expect, jest } from "@jest/globals";
 import { modules, services } from "../../utils/data";
 import Disable from "../../../src/commands/development/disable";
+import { Inventory } from "../../../src/state/inventory";
+import { StateManager } from "../../../src/state/state-manager";
 import { Config } from "@oclif/core";
+import Module from "../../../src/model/Module";
+import { DockerCompose } from "../../../src/run/docker-compose";
 
 const excludeServiceFromLiveUpdateMock = jest.fn();
 let snapshot;
 
-jest.mock("../../../src/state/inventory", () => {
-    return {
-        Inventory: function () {
-            return {
-                services,
-                modules
-            };
-        }
-    };
-});
+const inventoryMock = {
+    services,
+    modules: modules as Module[]
+};
 
-jest.mock("../../../src/state/state-manager", () => {
-    return {
-        StateManager: function () {
-            return {
-                snapshot,
-                excludeServiceFromLiveUpdate: excludeServiceFromLiveUpdateMock
-            };
-        }
-    };
-});
+const stateManagerMock = {
+    snapshot,
+    excludeServiceFromLiveUpdate: excludeServiceFromLiveUpdateMock
+};
+
+const dockerComposeMock = {
+    pull: jest.fn()
+};
+
+jest.mock("../../../src/state/inventory");
+
+jest.mock("../../../src/state/state-manager");
+
+jest.mock("../../../src/run/docker-compose");
 
 describe("development disable", () => {
     const cwdSpy = jest.spyOn(process, "cwd");
@@ -41,6 +43,15 @@ describe("development disable", () => {
 
         cwdSpy.mockReturnValue("./project");
 
+        // @ts-expect-error
+        Inventory.mockReturnValue(inventoryMock);
+
+        // @ts-expect-error
+        StateManager.mockReturnValue(stateManagerMock);
+
+        // @ts-expect-error
+        DockerCompose.mockReturnValue(dockerComposeMock);
+
         developmentDisable = new Disable(
             // @ts-expect-error
             [], { cacheDir: "./caches", runHook: runHookMock } as Config
@@ -60,6 +71,9 @@ describe("development disable", () => {
                 command: "disable",
                 services: ""
             },
+            flags: {
+                noPull: false
+            },
             argv: []
         });
 
@@ -75,6 +89,9 @@ describe("development disable", () => {
             args: {
                 services: ""
             },
+            flags: {
+                noPull: false
+            },
             argv: [
                 "",
                 "",
@@ -95,6 +112,9 @@ describe("development disable", () => {
                 command: "disable",
                 services: serviceName
             },
+            flags: {
+                noPull: false
+            },
             argv: [
                 serviceName
             ]
@@ -111,6 +131,9 @@ describe("development disable", () => {
             args: {
                 services: "service-two"
             },
+            flags: {
+                noPull: false
+            },
             argv: ["service-two"]
         });
 
@@ -123,4 +146,40 @@ describe("development disable", () => {
         expect(excludeServiceFromLiveUpdateMock).toHaveBeenCalledWith("service-two");
 
     });
+
+    it("runs docker compose pull", async () => {
+        // @ts-expect-error
+        parseMock.mockResolvedValue({
+            args: {
+                services: "service-two"
+            },
+            flags: {
+                noPull: false
+            },
+            argv: ["service-two"]
+        });
+
+        await expect(developmentDisable.run()).resolves.toBeUndefined();
+
+        expect(dockerComposeMock.pull).toHaveBeenCalledWith(
+            "service-two"
+        );
+    });
+
+    it("does not run docker compose pull when noPull true", async () => {
+        // @ts-expect-error
+        parseMock.mockResolvedValue({
+            args: {
+                services: "service-two"
+            },
+            flags: {
+                noPull: true
+            },
+            argv: ["service-two"]
+        });
+
+        await expect(developmentDisable.run()).resolves.toBeUndefined();
+
+        expect(dockerComposeMock.pull).not.toHaveBeenCalled();
+    });
 });

test/run/docker-compose.spec.ts

@@ -551,4 +551,37 @@ describe("DockerCompose", () => {
             ], expect.anything());
         });
     });
+
+    describe("pull", () => {
+
+        let dockerCompose;
+        const mockStdout = jest.fn();
+
+        const mockSterr = jest.fn();
+        const mockOnce = jest.fn();
+
+        beforeEach(() => {
+            jest.resetAllMocks();
+            dockerCompose = new DockerCompose(config, logger);
+
+            spawnMock.mockResolvedValue(undefined as never);
+
+        });
+
+        it("pulls the service", async () => {
+            await dockerCompose.pull(
+                "my-awesome-service"
+            );
+
+            expect(spawnMock).toHaveBeenCalledWith(
+                "docker",
+                [
+                    "compose",
+                    "pull",
+                    "my-awesome-service"
+                ],
+                expect.anything()
+            );
+        });
+    });
 });

test/run/logs/LogNothingLogHandler.spec.ts

@@ -0,0 +1,62 @@
+import { expect, jest } from "@jest/globals";
+import fs from "fs";
+import LogNothingLogHandler from "../../../src/run/logs/LogNothingLogHandler";
+
+describe("LogNothingLogHandler", () => {
+
+    const writeFileSyncSpy = jest.spyOn(fs, "writeFileSync");
+    const logFile = "/home/user/project/something.log";
+
+    beforeEach(() => {
+        jest.resetAllMocks();
+
+        writeFileSyncSpy.mockImplementation((_, __) => {});
+    });
+
+    it("does not log any lines", () => {
+        const loggerMock = {
+            log: jest.fn()
+        };
+
+        const logNothingLogHandler = new LogNothingLogHandler(logFile, loggerMock);
+
+        logNothingLogHandler.handle("one\ntwo\n\tthree\n");
+
+        expect(loggerMock.log).not.toHaveBeenCalled();
+    });
+
+    it("does write log entries to file", () => {
+        const logLines = [
+            "Watch configuration for service \"overseas-application\"",
+            "- Action rebuild for path \"/do\"",
+            "another log entry"
+        ];
+        const logMessage = logLines.join("\n");
+        const loggerMock = {
+            log: jest.fn()
+        };
+
+        const logNothingLogHandler = new LogNothingLogHandler(logFile, loggerMock);
+
+        logNothingLogHandler.handle(logMessage);
+
+        expect(writeFileSyncSpy).toHaveBeenCalledTimes(1);
+
+        const call = writeFileSyncSpy.mock.calls[0];
+
+        expect(call[0]).toBe(logFile);
+        expect(call[2]).toEqual({
+            flag: "a"
+        });
+
+        const contentsWrittenToFile = call[1];
+
+        for (const expectedLine of logLines) {
+            // eslint-disable-next-line no-useless-escape
+            const pattern = new RegExp(`\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}\.\\d{3}Z\\s-\\s${expectedLine}`, "g");
+
+            expect(pattern.test(contentsWrittenToFile.toString())).toBe(true);
+        }
+    });
+
+});