Skip to content

Commit

Permalink
Fix bug using --only filter to disable no-op deploy fails deploy beca…
Browse files Browse the repository at this point in the history
…use function source isn't uploaded (#5280)

Fixes #5270.
  • Loading branch information
taeold committed Dec 1, 2022
1 parent 6ed818b commit d59f265
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -3,3 +3,4 @@
- Fix bug in auth emulator in which createdAt was not set for signInWithIdp new users. (#5203)
- Default to --no-localhost when calling login from Google Cloud Workstations
- Support the x-goog-api-key header in auth emulator. (#5249)
- Fix bug where function deployments using --only filter sometimes failed deployments. (#5280)
8 changes: 7 additions & 1 deletion src/deploy/functions/deploy.ts
Expand Up @@ -112,7 +112,7 @@ export async function deploy(
await checkHttpIam(context, options, payload);
const uploads: Promise<void>[] = [];
for (const [codebase, { wantBackend, haveBackend }] of Object.entries(payload.functions)) {
if (shouldUploadBeSkipped(wantBackend, haveBackend)) {
if (shouldUploadBeSkipped(context, wantBackend, haveBackend)) {
continue;
}
uploads.push(uploadCodebase(context, codebase, wantBackend));
Expand All @@ -124,9 +124,15 @@ export async function deploy(
* @return True IFF wantBackend + haveBackend are the same
*/
export function shouldUploadBeSkipped(
context: args.Context,
wantBackend: backend.Backend,
haveBackend: backend.Backend
): boolean {
// If function targets are specified by --only flag, assume that function will be deployed
// and go ahead and upload the source.
if (context.filters && context.filters.length > 0) {
return false;
}
const wantEndpoints = backend.allEndpoints(wantBackend);
const haveEndpoints = backend.allEndpoints(haveBackend);

Expand Down
31 changes: 27 additions & 4 deletions src/test/deploy/functions/deploy.spec.ts
@@ -1,5 +1,6 @@
import { expect } from "chai";

import * as args from "../../../deploy/functions/args";
import * as backend from "../../../deploy/functions/backend";
import * as deploy from "../../../deploy/functions/deploy";

Expand All @@ -17,6 +18,11 @@ describe("deploy", () => {
...ENDPOINT_BASE,
httpsTrigger: {},
};

const CONTEXT: args.Context = {
projectId: "project",
};

describe("shouldUploadBeSkipped", () => {
let endpoint1InWantBackend: backend.Endpoint;
let endpoint2InWantBackend: backend.Endpoint;
Expand Down Expand Up @@ -63,7 +69,7 @@ describe("deploy", () => {
endpoint2InHaveBackend.hash = endpoint2InWantBackend.hash;

// Execute
const result = deploy.shouldUploadBeSkipped(wantBackend, haveBackend);
const result = deploy.shouldUploadBeSkipped(CONTEXT, wantBackend, haveBackend);

// Expect
expect(result).to.be.true;
Expand All @@ -76,7 +82,7 @@ describe("deploy", () => {
endpoint2InHaveBackend.hash = "No_match";

// Execute
const result = deploy.shouldUploadBeSkipped(wantBackend, haveBackend);
const result = deploy.shouldUploadBeSkipped(CONTEXT, wantBackend, haveBackend);

// Expect
expect(result).to.be.false;
Expand All @@ -92,7 +98,7 @@ describe("deploy", () => {
haveBackend = backend.of(endpoint1InHaveBackend);

// Execute
const result = deploy.shouldUploadBeSkipped(wantBackend, haveBackend);
const result = deploy.shouldUploadBeSkipped(CONTEXT, wantBackend, haveBackend);

// Expect
expect(result).to.be.false;
Expand All @@ -108,7 +114,24 @@ describe("deploy", () => {
haveBackend = backend.of(endpoint1InHaveBackend, endpoint2InHaveBackend);

// Execute
const result = deploy.shouldUploadBeSkipped(wantBackend, haveBackend);
const result = deploy.shouldUploadBeSkipped(CONTEXT, wantBackend, haveBackend);

// Expect
expect(result).to.be.false;
});

it("should not skip if endpoint filter is specified", () => {
endpoint1InWantBackend.hash = "1";
endpoint2InWantBackend.hash = "2";
endpoint1InHaveBackend.hash = endpoint1InWantBackend.hash;
endpoint2InHaveBackend.hash = endpoint2InWantBackend.hash;

// Execute
const result = deploy.shouldUploadBeSkipped(
{ ...CONTEXT, filters: [{ idChunks: ["foobar"] }] },
wantBackend,
haveBackend
);

// Expect
expect(result).to.be.false;
Expand Down

0 comments on commit d59f265

Please sign in to comment.