Skip to content

[AI Improvement] [Task] Await chai-as-promised assertions in 6 spec files that never run today (lint: no-floating-promises, spec slice) #11469

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
joehan wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[AI Improvement] [Task] Await chai-as-promised assertions in 6 spec files that never run today (lint: no-floating-promises, spec slice) #11469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/database/fakeRemoveRemote.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,23 @@
import { RemoveRemote } from "./removeRemote";

export class FakeRemoveRemote implements RemoveRemote {
data: any;

Check warning on line 7 in src/database/fakeRemoveRemote.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24) 

Unexpected any. Specify a different type
largeThreshold: number;

/**
* @constructor

Check warning on line 11 in src/database/fakeRemoveRemote.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24) 

Invalid JSDoc tag (preference). Replace "constructor" JSDoc tag with "class"
* @param data the fake database structure. Each leaf is an integer representing the subtree's size.
* @param largeThreshold the threshold to determine if a delete exceeds the writeSizeLimit.
* If the sum of all leaves to delete is larger than largeThreshold,
* the delete will return false.
*/
constructor(data: any, largeThreshold = 10) {

Check warning on line 17 in src/database/fakeRemoveRemote.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24) 

Unexpected any. Specify a different type
this.data = data;

Check warning on line 18 in src/database/fakeRemoveRemote.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24) 

Unsafe assignment of an `any` value
this.largeThreshold = largeThreshold;
}

deletePath(path: string): Promise<boolean> {
const d = this._dataAtpath(path);

Check warning on line 23 in src/database/fakeRemoveRemote.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24) 

Unsafe assignment of an `any` value
const size = this._size(d);
if (size > this.largeThreshold) {
return Promise.resolve(false);
Expand All @@ -29,19 +29,19 @@
return Promise.resolve(true);
}

deleteSubPath(path: string, subPaths: string[]): Promise<boolean> {
async deleteSubPath(path: string, subPaths: string[]): Promise<boolean> {
const d = this._dataAtpath(path);

Check warning on line 33 in src/database/fakeRemoveRemote.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24) 

Unsafe assignment of an `any` value
let size = 0;
for (const p of subPaths) {
size += this._size(d[p]);

Check warning on line 36 in src/database/fakeRemoveRemote.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24) 

Unsafe member access [p] on an `any` value
}
Comment on lines +32 to 37

@gemini-code-assist gemini-code-assist Bot Nov 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If d is null/undefined, or if a subpath does not exist in d (making d[p] undefined), accessing properties or calling _size will throw a TypeError. Adding defensive checks ensures the fake database helper is robust against non-existent paths.

  async deleteSubPath(path: string, subPaths: string[]): Promise<boolean> {
    const d = this._dataAtpath(path);
    if (!d) {
      return true;
    }
    let size = 0;
    for (const p of subPaths) {
      if (d[p] !== undefined) {
        size += this._size(d[p]);
      }
    }
References
  1. Use strict null checks and handle undefined/null explicitly. (link)

if (size > this.largeThreshold) {
return Promise.resolve(false);
return false;
}
for (const p of subPaths) {
this.deletePath(`${path}/${p}`);
await this.deletePath(`${path}/${p}`);
}
return Promise.resolve(true);
return true;
}

private _deletePath(path: string): void {
Expand All @@ -51,13 +51,13 @@
}
const parentDir = pathLib.dirname(path);
const basename = pathLib.basename(path);
delete this._dataAtpath(parentDir)[basename];

Check warning on line 54 in src/database/fakeRemoveRemote.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24) 

Unsafe member access [basename] on an `any` value
if (Object.keys(this._dataAtpath(parentDir)).length === 0) {

Check warning on line 55 in src/database/fakeRemoveRemote.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24) 

Unsafe argument of type `any` assigned to a parameter of type `{}`
return this._deletePath(parentDir);
}
}

private _size(data: any): number {

Check warning on line 60 in src/database/fakeRemoveRemote.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24) 

Unexpected any. Specify a different type
if (typeof data === "number") {
return data;
}
Expand Down
6 changes: 3 additions & 3 deletions src/deploy/extensions/planner.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ describe("Extensions Deployment Planner", () => {
];

for (const c of cases) {
it(c.description, () => {
it(c.description, async () => {
if (!c.err) {
expect(
await expect(
planner.resolveVersion(
{
publisherId: "test",
Expand All @@ -99,7 +99,7 @@ describe("Extensions Deployment Planner", () => {
),
).to.eventually.equal(c.out);
} else {
expect(
await expect(
planner.resolveVersion(
{
publisherId: "test",
Expand Down
2 changes: 1 addition & 1 deletion src/deploy/functions/remoteSource.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describe("remoteSource", () => {
const entryPath = topLevelDir ? path.join(topLevelDir, filePath) : filePath;
archive.append(content, { name: entryPath });
}
archive.finalize();
archive.finalize().catch(reject);
});
}

Expand Down
20 changes: 10 additions & 10 deletions src/deploy/functions/validate.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -767,19 +767,19 @@ describe("validate", () => {
secretVersionStub.restore();
});

it("passes validation with empty backend", () => {
expect(validate.secretsAreValid(project, backend.empty())).to.not.be.rejected;
it("passes validation with empty backend", async () => {
await expect(validate.secretsAreValid(project, backend.empty())).to.not.be.rejected;
});

it("passes validation with no secret env vars", () => {
it("passes validation with no secret env vars", async () => {
const b = backend.of({
...ENDPOINT,
platform: "gcfv2",
});
expect(validate.secretsAreValid(project, b)).to.not.be.rejected;
await expect(validate.secretsAreValid(project, b)).to.not.be.rejected;
});

it("fails validation given non-existent secret version", () => {
it("fails validation given non-existent secret version", async () => {
secretVersionStub.rejects({ reason: "Secret version does not exist" });

const b = backend.of({
Expand All @@ -793,13 +793,13 @@ describe("validate", () => {
},
],
});
expect(validate.secretsAreValid(project, b)).to.be.rejectedWith(
await expect(validate.secretsAreValid(project, b)).to.be.rejectedWith(
FirebaseError,
/Failed to validate secret version/,
);
});

it("fails validation given non-existent secret version", () => {
it("fails validation given non-existent secret version", async () => {
secretVersionStub.rejects({ reason: "Secret version does not exist" });

const b = backend.of({
Expand All @@ -813,13 +813,13 @@ describe("validate", () => {
},
],
});
expect(validate.secretsAreValid(project, b)).to.be.rejectedWith(
await expect(validate.secretsAreValid(project, b)).to.be.rejectedWith(
FirebaseError,
/Failed to validate secret versions/,
);
});

it("fails validation given disabled secret version", () => {
it("fails validation given disabled secret version", async () => {
secretVersionStub.resolves({
secret,
versionId: "1",
Expand All @@ -837,7 +837,7 @@ describe("validate", () => {
},
],
});
expect(validate.secretsAreValid(project, b)).to.be.rejectedWith(
await expect(validate.secretsAreValid(project, b)).to.be.rejectedWith(
FirebaseError,
/Failed to validate secret versions/,
);
Expand Down
6 changes: 4 additions & 2 deletions src/emulator/workQueue.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ describe("WorkQueue", () => {

beforeEach(() => {
queue = new WorkQueue(FunctionsExecutionMode.AUTO, MAX_PARALLEL);
queue.start();
// Runs until afterEach calls stop().
void queue.start();
});

afterEach(() => {
Expand Down Expand Up @@ -90,7 +91,8 @@ describe("WorkQueue", () => {
let queue: WorkQueue;
beforeEach(() => {
queue = new WorkQueue(FunctionsExecutionMode.SEQUENTIAL);
queue.start();
// Runs until afterEach calls stop().
void queue.start();
});

afterEach(() => {
Expand Down
6 changes: 3 additions & 3 deletions src/throttler/throttler.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ const throttlerTest = (ThrottlerConstructor: ThrottlerConstructorType): void =>
});
});

it("should return the result of task", () => {
it("should return the result of task", async () => {
const handler = (task: number) => {
return Promise.resolve(`result: ${task}`);
};
Expand All @@ -244,8 +244,8 @@ const throttlerTest = (ThrottlerConstructor: ThrottlerConstructorType): void =>
handler,
});

expect(q.run(2)).to.eventually.to.equal("result: 2");
expect(q.run(3)).to.eventually.to.equal("result: 3");
await expect(q.run(2)).to.eventually.to.equal("result: 2");
await expect(q.run(3)).to.eventually.to.equal("result: 3");
Comment on lines +247 to +248

@gemini-code-assist gemini-code-assist Bot Nov 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The assertion chain contains a redundant .to after .eventually. It should be simplified to .to.eventually.equal for better readability and consistency with other assertions in the codebase.

Suggested change
await expect(q.run(2)).to.eventually.to.equal("result: 2");
await expect(q.run(3)).to.eventually.to.equal("result: 3");
await expect(q.run(2)).to.eventually.equal("result: 2");
await expect(q.run(3)).to.eventually.equal("result: 3");

});

it("should resolve if task finishes before timeout", async () => {
Expand Down
Loading