Skip to content

Commit 94cefb3

Browse files
committed
fixed after review
1 parent b9340fa commit 94cefb3

8 files changed

+16
-119
lines changed

packages/modules/gcloud/src/datastore-emulator-container.test.ts

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { Datastore } from "@google-cloud/datastore";
2-
import { Wait } from "testcontainers";
32
import { DatastoreEmulatorContainer, StartedDatastoreEmulatorContainer } from "./datastore-emulator-container";
43

54
describe("DatastoreEmulatorContainer", { timeout: 240_000 }, () => {
@@ -26,35 +25,6 @@ describe("DatastoreEmulatorContainer", { timeout: 240_000 }, () => {
2625

2726
// }
2827

29-
it("should have default host-port flag and database-mode flag", async () => {
30-
const datastoreEmulatorContainer = new DatastoreEmulatorContainer();
31-
32-
const flags = datastoreEmulatorContainer["flagsManager"].expandFlags();
33-
34-
expect(flags.trim()).toEqual("--host-port=0.0.0.0:8080 --database-mode=datastore-mode");
35-
});
36-
37-
it("should be able to add flags after creating container", async () => {
38-
const datastoreEmulatorContainer = new DatastoreEmulatorContainer();
39-
// clear all default flags
40-
datastoreEmulatorContainer["flagsManager"].clearFlags();
41-
42-
// add some new flags
43-
const flags = datastoreEmulatorContainer
44-
.withFlag("host-port", "0.0.0.0:8080")
45-
.withFlag("database-mode", "datastore-mode")
46-
["flagsManager"].expandFlags();
47-
48-
// check new added flags exists
49-
expect(flags.trim()).toEqual("--host-port=0.0.0.0:8080 --database-mode=datastore-mode");
50-
51-
// check that container start command uses latest flags string
52-
const startedContainer = await datastoreEmulatorContainer
53-
.withWaitStrategy(Wait.forLogMessage(/.* start --host=0.0.0.0 --port=8080 --database-mode=datastore-mode/, 1))
54-
.start();
55-
await startedContainer.stop();
56-
});
57-
5828
async function checkDatastore(datastoreEmulatorContainer: StartedDatastoreEmulatorContainer) {
5929
expect(datastoreEmulatorContainer).toBeDefined();
6030
const testProjectId = "test-project";

packages/modules/gcloud/src/datastore-emulator-container.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,7 @@ export class DatastoreEmulatorContainer extends GenericContainer {
1313
this.withExposedPorts(EMULATOR_PORT)
1414
.withFlag("host-port", `0.0.0.0:${EMULATOR_PORT}`)
1515
.withFlag("database-mode", `datastore-mode`)
16-
// explicitly call withCommand() fn here
17-
// it will be called implicitly inside prev withFlag() call
18-
.withCommand(["/bin/sh", "-c", this.getCmd()])
19-
.withWaitStrategy(Wait.forLogMessage(RegExp(".*running.*"), 1))
16+
.withWaitStrategy(Wait.forLogMessage(/.*running.*/, 1))
2017
.withStartupTimeout(120_000);
2118
}
2219

@@ -37,12 +34,12 @@ export class DatastoreEmulatorContainer extends GenericContainer {
3734
*/
3835
public withFlag(name: string, value: string) {
3936
this.flagsManager.withFlag(name, value);
40-
// we need to 'refresh' command as we add new flag
41-
this.withCommand(["/bin/sh", "-c", this.getCmd()]);
4237
return this;
4338
}
4439

4540
public override async start(): Promise<StartedDatastoreEmulatorContainer> {
41+
// expand all flags and get final command
42+
this.withCommand(["/bin/sh", "-c", this.getCmd()]);
4643
return new StartedDatastoreEmulatorContainer(await super.start());
4744
}
4845
}

packages/modules/gcloud/src/emulator-flags-manager.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ describe("EmulatorFlagsManager", () => {
4242

4343
const flags = flagsManager.expandFlags();
4444

45-
expect(flags.trim()).toEqual("--database-mode= --host-port=");
45+
expect(flags.trim()).toEqual("--database-mode --host-port");
4646
});
4747

4848
it("should throw if flag name not set", async () => {
Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
export class EmulatorFlagsManager {
2-
private flags: { name: string; value: string }[] = [];
2+
private flags: { [name: string]: string } = {};
33

44
/**
55
* Adds flag as argument to emulator start command.
@@ -10,33 +10,27 @@ export class EmulatorFlagsManager {
1010
*/
1111
public withFlag(name: string, value: string): this {
1212
if (!name) throw new Error("Flag name must be set.");
13-
// replace flag if it already exists
14-
const idx = this.flags.findIndex((f) => f.name == this.trimFlagName(name));
15-
if (idx >= 0) this.flags = [...this.flags.slice(0, idx), ...this.flags.slice(idx + 1)];
16-
this.flags.push({ name, value });
13+
if (name.startsWith("--")) this.flags[name] = value;
14+
else this.flags[`--${name}`] = value;
1715
return this;
1816
}
1917

20-
private trimFlagName(name: string): string {
21-
return name?.startsWith("--") ? name.slice(2) : name;
22-
}
23-
24-
private flagToString(f: { name: string; value: string }): string {
25-
return `--${this.trimFlagName(f.name)}=${f.value}`;
18+
private flagToString(name: string, value: string): string {
19+
return `${name}${value ? "=" + value : ""}`;
2620
}
2721

2822
/**
2923
*
3024
* @returns string with all flag names and values, concatenated in same order they were added.
3125
*/
3226
public expandFlags(): string {
33-
return `${this.flags.reduce((p, c) => p + " " + this.flagToString(c), "")}`;
27+
return `${Object.keys(this.flags).reduce((p, c) => p + " " + this.flagToString(c, this.flags[c]), "")}`;
3428
}
3529

3630
/**
3731
* Clears all added flags.
3832
*/
3933
public clearFlags() {
40-
this.flags = [];
34+
this.flags = {};
4135
}
4236
}

packages/modules/gcloud/src/firestore-emulator-container.test.ts

Lines changed: 1 addition & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
import * as admin from "firebase-admin";
2-
import { Wait } from "testcontainers";
32
import { FirestoreEmulatorContainer, StartedFirestoreEmulatorContainer } from "./firestore-emulator-container";
43

54
describe("FirestoreEmulatorContainer", { timeout: 240_000 }, () => {
6-
afterEach(async (ctx) => {
7-
if (!["should work using default version", "should work using version 468.0.0"].includes(ctx.task.name)) return;
5+
afterEach(async () => {
86
await admin.app().delete();
97
});
108

@@ -31,35 +29,6 @@ describe("FirestoreEmulatorContainer", { timeout: 240_000 }, () => {
3129

3230
// }
3331

34-
it("should have default host-port flag", async () => {
35-
const firestoreEmulatorContainer = new FirestoreEmulatorContainer();
36-
37-
const flags = firestoreEmulatorContainer["flagsManager"].expandFlags();
38-
39-
expect(flags.trim()).toEqual("--host-port=0.0.0.0:8080");
40-
});
41-
42-
it("should be able to add flags after creating container", async () => {
43-
const firestoreEmulatorContainer = new FirestoreEmulatorContainer();
44-
// clear all default flags
45-
firestoreEmulatorContainer["flagsManager"].clearFlags();
46-
47-
// add some new flags
48-
const flags = firestoreEmulatorContainer
49-
.withFlag("host-port", "0.0.0.0:8080")
50-
.withFlag("database-mode", "datastore-mode")
51-
["flagsManager"].expandFlags();
52-
53-
// check new added flags exists
54-
expect(flags.trim()).toEqual("--host-port=0.0.0.0:8080 --database-mode=datastore-mode");
55-
56-
// check that container start command uses latest flags string
57-
const startedContainer = await firestoreEmulatorContainer
58-
.withWaitStrategy(Wait.forLogMessage(/.* start --host=0.0.0.0 --port=8080 --database-mode=datastore-mode/, 1))
59-
.start();
60-
await startedContainer.stop();
61-
});
62-
6332
async function checkFirestore(firestoreEmulatorContainer: StartedFirestoreEmulatorContainer) {
6433
expect(firestoreEmulatorContainer).toBeDefined();
6534
const testProjectId = "test-project";

packages/modules/gcloud/src/firestore-emulator-container.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,7 @@ export class FirestoreEmulatorContainer extends GenericContainer {
1212
this._flagsManager = new EmulatorFlagsManager();
1313
this.withExposedPorts(EMULATOR_PORT)
1414
.withFlag("host-port", `0.0.0.0:${EMULATOR_PORT}`)
15-
// explicitly call withCommand() fn here
16-
// it will be called implicitly inside prev withFlag() call
17-
.withCommand(["/bin/sh", "-c", this.getCmd()])
18-
.withWaitStrategy(Wait.forLogMessage(RegExp(".*running.*"), 1))
15+
.withWaitStrategy(Wait.forLogMessage(/.*running.*/, 1))
1916
.withStartupTimeout(120_000);
2017
}
2118

@@ -36,12 +33,12 @@ export class FirestoreEmulatorContainer extends GenericContainer {
3633
*/
3734
public withFlag(name: string, value: string) {
3835
this.flagsManager.withFlag(name, value);
39-
// we need to 'refresh' command as we add new flag
40-
this.withCommand(["/bin/sh", "-c", this.getCmd()]);
4136
return this;
4237
}
4338

4439
public override async start(): Promise<StartedFirestoreEmulatorContainer> {
40+
// expand all flags and get final command
41+
this.withCommand(["/bin/sh", "-c", this.getCmd()]);
4542
return new StartedFirestoreEmulatorContainer(await super.start());
4643
}
4744
}

packages/modules/gcloud/src/pubsub-emulator-container.test.ts

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { PubSub } from "@google-cloud/pubsub";
2-
import { Wait } from "testcontainers";
32
import { PubSubEmulatorContainer, StartedPubSubEmulatorContainer } from "./pubsub-emulator-container";
43

54
describe("PubSubEmulatorContainer", { timeout: 240_000 }, () => {
@@ -11,32 +10,6 @@ describe("PubSubEmulatorContainer", { timeout: 240_000 }, () => {
1110
await pubsubEmulatorContainer.stop();
1211
});
1312

14-
it("should have default host-port flag", async () => {
15-
const pubsubEmulatorContainer = new PubSubEmulatorContainer();
16-
17-
const flags = pubsubEmulatorContainer["flagsManager"].expandFlags();
18-
19-
expect(flags.trim()).toEqual("--host-port=0.0.0.0:8085");
20-
});
21-
22-
it("should be able to add flags after creating container", async () => {
23-
const pubsubEmulatorContainer = new PubSubEmulatorContainer();
24-
// clear all default flags
25-
pubsubEmulatorContainer["flagsManager"].clearFlags();
26-
27-
// add some new flags
28-
const flags = pubsubEmulatorContainer.withFlag("host-port", "0.0.0.0:8081")["flagsManager"].expandFlags();
29-
30-
// check new added flags exists
31-
expect(flags.trim()).toEqual("--host-port=0.0.0.0:8081");
32-
33-
// check that container start command uses latest flags string
34-
const startedContainer = await pubsubEmulatorContainer
35-
.withWaitStrategy(Wait.forLogMessage(/.* listening on 8081/, 1))
36-
.start();
37-
await startedContainer.stop();
38-
});
39-
4013
async function checkPubSub(pubsubEmulatorContainer: StartedPubSubEmulatorContainer) {
4114
expect(pubsubEmulatorContainer).toBeDefined();
4215

packages/modules/gcloud/src/pubsub-emulator-container.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,6 @@ export class PubSubEmulatorContainer extends GenericContainer {
3535
*/
3636
public withFlag(name: string, value: string) {
3737
this.flagsManager.withFlag(name, value);
38-
// we need to 'refresh' command as we add new flag
39-
this.withCommand(["/bin/sh", "-c", this.getCmd()]);
4038
return this;
4139
}
4240

@@ -49,8 +47,7 @@ export class PubSubEmulatorContainer extends GenericContainer {
4947
// Determine the valid command-line prompt when starting the Pub/Sub emulator
5048
const selectedProjectId = this._projectId ?? "test-project";
5149
this.withFlag("project", selectedProjectId)
52-
// explicitly call withCommand() fn here
53-
// it will be called implicitly inside prev withFlag() call
50+
// expand all flags and get final command
5451
.withCommand(["/bin/sh", "-c", this.getCmd()]);
5552

5653
return new StartedPubSubEmulatorContainer(await super.start(), selectedProjectId);

0 commit comments

Comments
 (0)