From 488f550cde3a757205e92d58c7b4b313b71ef796 Mon Sep 17 00:00:00 2001 From: Mathusan Selvarajah Date: Wed, 19 Feb 2025 18:34:51 +0000 Subject: [PATCH 1/5] fix bug where apphosting emulator info is not complete when env vars for other emulators are set --- src/emulator/apphosting/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/emulator/apphosting/index.ts b/src/emulator/apphosting/index.ts index 20d989b4582..d8da1f5a927 100644 --- a/src/emulator/apphosting/index.ts +++ b/src/emulator/apphosting/index.ts @@ -39,8 +39,8 @@ export class AppHostingEmulator implements EmulatorInstance { getInfo(): EmulatorInfo { return { name: Emulators.APPHOSTING, - host: this.args.options.host!, - port: this.args.options.port!, + host: this.args.options.host ?? this.args.host, + port: this.args.options.port ?? this.args.port, }; } From bc032fc40f5e1fcdf9940d87a21ccb0556ee9e83 Mon Sep 17 00:00:00 2001 From: Mathusan Selvarajah Date: Wed, 19 Feb 2025 19:22:23 +0000 Subject: [PATCH 2/5] add proper fix and test --- src/emulator/apphosting/index.ts | 4 ++-- src/emulator/apphosting/serve.spec.ts | 20 ++++++++++++++++++++ src/emulator/apphosting/serve.ts | 10 ++++++++-- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/emulator/apphosting/index.ts b/src/emulator/apphosting/index.ts index d8da1f5a927..bbd1c3f0fa7 100644 --- a/src/emulator/apphosting/index.ts +++ b/src/emulator/apphosting/index.ts @@ -39,8 +39,8 @@ export class AppHostingEmulator implements EmulatorInstance { getInfo(): EmulatorInfo { return { name: Emulators.APPHOSTING, - host: this.args.options.host ?? this.args.host, - port: this.args.options.port ?? this.args.port, + host: this.args.options.host, + port: this.args.options.port, }; } diff --git a/src/emulator/apphosting/serve.spec.ts b/src/emulator/apphosting/serve.spec.ts index 819c080b3f1..da6d9c700d4 100644 --- a/src/emulator/apphosting/serve.spec.ts +++ b/src/emulator/apphosting/serve.spec.ts @@ -8,6 +8,8 @@ import * as utils from "./developmentServer"; import * as configsImport from "./config"; import * as projectPathImport from "../../projectPath"; import { AppHostingYamlConfig } from "../../apphosting/yaml"; +import * as emulatorRegistry from "../registry"; +import * as emulatorEnvs from "../env"; describe("serve", () => { let checkListenableStub: sinon.SinonStub; @@ -16,6 +18,8 @@ describe("serve", () => { let detectStartCommandStub: sinon.SinonStub; let configsStub: sinon.SinonStubbedInstance; let resolveProjectPathStub: sinon.SinonStub; + let listRunningWithInfoStub: sinon.SinonStub; + let setEnvVarsForEmulatorsStub: sinon.SinonStub; beforeEach(() => { checkListenableStub = sinon.stub(portUtils, "checkListenable"); @@ -25,6 +29,9 @@ describe("serve", () => { configsStub = sinon.stub(configsImport); resolveProjectPathStub = sinon.stub(projectPathImport, "resolveProjectPath"); + listRunningWithInfoStub = sinon.stub(emulatorRegistry.EmulatorRegistry, "listRunningWithInfo"); + setEnvVarsForEmulatorsStub = sinon.stub(emulatorEnvs, "setEnvVarsForEmulators"); + resolveProjectPathStub.returns(""); detectStartCommandStub.returns("npm run dev"); }); @@ -37,6 +44,10 @@ describe("serve", () => { }); describe("start", () => { + beforeEach(() => { + listRunningWithInfoStub.returns([]); + }); + it("should use user-provided port if one is defined", async () => { checkListenableStub.onFirstCall().returns(true); configsStub.getLocalAppHostingConfiguration.returns( @@ -71,4 +82,13 @@ describe("serve", () => { expect(spawnWithCommandStringStub.getCall(0).args[0]).to.eq(startCommand); }); }); + + describe("getEmulatorEnvs", async () => { + it("should omit apphosting emulator", async () => { + listRunningWithInfoStub.returns([{ name: "apphosting" }, { name: "functions" }]); + serve.getEmulatorEnvs(); + + expect(setEnvVarsForEmulatorsStub).to.be.calledWith({}, [{ name: "functions" }]); + }); + }); }); diff --git a/src/emulator/apphosting/serve.ts b/src/emulator/apphosting/serve.ts index 3e31a0dcdf3..d1995441efa 100644 --- a/src/emulator/apphosting/serve.ts +++ b/src/emulator/apphosting/serve.ts @@ -85,9 +85,15 @@ function availablePort(host: string, port: number): Promise { }); } -function getEmulatorEnvs(): Record { +/** + * Exported for unit tests + */ +export function getEmulatorEnvs(): Record { const envs: Record = {}; - const emulatorInfos = EmulatorRegistry.listRunningWithInfo(); + // No need to set envs for the apphosting emulator itself. + const emulatorInfos = EmulatorRegistry.listRunningWithInfo().filter( + (emulator) => emulator.name !== "apphosting", + ); setEnvVarsForEmulators(envs, emulatorInfos); return envs; From 15266b7ba78b8b8289072dfa95712e41c2a2c919 Mon Sep 17 00:00:00 2001 From: Mathusan Selvarajah Date: Wed, 19 Feb 2025 19:28:26 +0000 Subject: [PATCH 3/5] fix --- src/emulator/apphosting/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/emulator/apphosting/index.ts b/src/emulator/apphosting/index.ts index bbd1c3f0fa7..20d989b4582 100644 --- a/src/emulator/apphosting/index.ts +++ b/src/emulator/apphosting/index.ts @@ -39,8 +39,8 @@ export class AppHostingEmulator implements EmulatorInstance { getInfo(): EmulatorInfo { return { name: Emulators.APPHOSTING, - host: this.args.options.host, - port: this.args.options.port, + host: this.args.options.host!, + port: this.args.options.port!, }; } From 0401ade16443ed92a48c568e56a6315d433d04fb Mon Sep 17 00:00:00 2001 From: Mathusan Selvarajah Date: Wed, 19 Feb 2025 19:39:00 +0000 Subject: [PATCH 4/5] remove async from non-async test func --- src/emulator/apphosting/serve.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/emulator/apphosting/serve.spec.ts b/src/emulator/apphosting/serve.spec.ts index da6d9c700d4..8a94a461ebe 100644 --- a/src/emulator/apphosting/serve.spec.ts +++ b/src/emulator/apphosting/serve.spec.ts @@ -83,8 +83,8 @@ describe("serve", () => { }); }); - describe("getEmulatorEnvs", async () => { - it("should omit apphosting emulator", async () => { + describe("getEmulatorEnvs", () => { + it("should omit apphosting emulator", () => { listRunningWithInfoStub.returns([{ name: "apphosting" }, { name: "functions" }]); serve.getEmulatorEnvs(); From 14433ad094030e7cb2a8a996d96e4eaa960a4201 Mon Sep 17 00:00:00 2001 From: Mathusan Selvarajah Date: Wed, 19 Feb 2025 21:42:30 +0000 Subject: [PATCH 5/5] address comments --- src/emulator/apphosting/serve.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/emulator/apphosting/serve.ts b/src/emulator/apphosting/serve.ts index d1995441efa..cc8760a8747 100644 --- a/src/emulator/apphosting/serve.ts +++ b/src/emulator/apphosting/serve.ts @@ -90,9 +90,8 @@ function availablePort(host: string, port: number): Promise { */ export function getEmulatorEnvs(): Record { const envs: Record = {}; - // No need to set envs for the apphosting emulator itself. const emulatorInfos = EmulatorRegistry.listRunningWithInfo().filter( - (emulator) => emulator.name !== "apphosting", + (emulator) => emulator.name !== Emulators.APPHOSTING, // No need to set envs for the apphosting emulator itself. ); setEnvVarsForEmulators(envs, emulatorInfos);