Skip to content

increase timeout for npm run test and factor in timezone day diff for datetimeString test #9812

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
aalej wants to merge 4 commits into
base: main
Choose a base branch
from
Open

increase timeout for npm run test and factor in timezone day diff for datetimeString test #9812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
007fcc3
increase timeout
aalej Jan 26, 2026
7164238
modify regex to factor day diffence due to timezone
aalej Jan 26, 2026
d0b7e87
make timeout 5s
aalej Jan 26, 2026
39015b5
correct regex mistake
aalej Jan 26, 2026
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
2 changes: 1 addition & 1 deletion .mocharc.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ require:
- src/test/helpers/mocha-bootstrap.ts
file:
- src/test/helpers/global-mock-auth.ts
timeout: 1000
timeout: 5000
recursive: true
node-options:
- no-experimental-strip-types
7 changes: 4 additions & 3 deletions src/utils.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
it("should return provided value if coerce fails", () => {
process.env.FOO_BAR_BAZ = "set";

const coerce = () => {

Check warning on line 69 in src/utils.spec.ts

View workflow job for this annotation

GitHub Actions / lint (20) 

Missing return type on function
throw new Error();
};
expect(utils.envOverride("FOO_BAR_BAZ", "notset", coerce)).to.deep.equal("notset");
Expand Down Expand Up @@ -206,7 +206,7 @@
it("should settle all promises", async () => {
const result = await utils.promiseAllSettled([
Promise.resolve("foo"),
Promise.reject("bar"),

Check warning on line 209 in src/utils.spec.ts

View workflow job for this annotation

GitHub Actions / lint (20) 

Expected the Promise rejection reason to be an Error
Promise.resolve("baz"),
]);
expect(result).to.deep.equal([
Expand All @@ -228,7 +228,7 @@
bar: Promise.resolve("2"),
};

const result = await utils.promiseProps(o);

Check warning on line 231 in src/utils.spec.ts

View workflow job for this annotation

GitHub Actions / lint (20) 

Unsafe assignment of an `any` value
expect(result).to.deep.equal({
foo: "1",
bar: "2",
Expand All @@ -245,7 +245,7 @@
bar: ["bar"],
};

const result = await utils.promiseProps(o);

Check warning on line 248 in src/utils.spec.ts

View workflow job for this annotation

GitHub Actions / lint (20) 

Unsafe assignment of an `any` value
expect(result).to.deep.equal({
foo: "1",
bar: ["bar"],
Expand All @@ -269,14 +269,15 @@
describe("datetimeString", () => {
it("should output the date in the correct format", () => {
// Don't worry about the hour since timezones screw everything up.
// Timezone differences may cause the day to be off by one, so we allow for that.
expect(utils.datetimeString(new Date("February 22, 2020 11:35:45-07:00"))).to.match(
/^2020-02-22 \d\d:35:45$/,
/^2020-02-2[23] \d\d:35:45$/,
);
expect(utils.datetimeString(new Date("February 7, 2020 11:35:45-07:00"))).to.match(
/^2020-02-07 \d\d:35:45$/,
/^2020-02-0[78] \d\d:35:45$/,
);
expect(utils.datetimeString(new Date("February 7, 2020 8:01:01-07:00"))).to.match(
/^2020-02-07 \d\d:01:01$/,
/^2020-02-0[78] \d\d:01:01$/,
);
Comment on lines 273 to 281
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 26, 2026

Choose a reason for hiding this comment

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

medium

While this change correctly fixes the test for different timezones by allowing for a day difference, it might not cover all edge cases, such as month or year boundaries. For example, a date like December 31, 2020 20:00:00Z could be 2021-01-01 in some timezones, causing both the day, month, and year to change.

To make the tests more robust, you could consider:

  • Mocking the timezone for these tests (which might require an external library).
  • Adding test cases for boundary conditions (e.g., end of month, end of year) to ensure datetimeString behaves as expected across timezone-induced date changes.

Copy link
Contributor Author

@aalej aalej Jan 27, 2026

Choose a reason for hiding this comment

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

For now, for this specific test, I think we can ignore this edge case? the purpose of the test is to check if dateTimeString util will correctly format a date object into a YYYY-mm-dd HH:mm:ss format

});
});
Expand Down Expand Up @@ -312,12 +313,12 @@
expect(res.length).to.equal(1);
expect(res[0].status).to.equal("rejected");
expect((res[0] as utils.PromiseRejectedResult).reason).to.be.instanceOf(Error);
expect((res[0] as any).reason?.message).to.equal("oh noes!");

Check warning on line 316 in src/utils.spec.ts

View workflow job for this annotation

GitHub Actions / lint (20) 

Unexpected any. Specify a different type

Check warning on line 316 in src/utils.spec.ts

View workflow job for this annotation

GitHub Actions / lint (20) 

Unsafe member access .reason on an `any` value
});

it("waits for all settled", async () => {
// Intetionally failing with a non-error to make matching easier
const reject = Promise.reject("fail fast");

Check warning on line 321 in src/utils.spec.ts

View workflow job for this annotation

GitHub Actions / lint (20) 

Expected the Promise rejection reason to be an Error
const resolve = new Promise((res) => {
setTimeout(() => res(42), 20);
});
Expand Down
Loading