Skip to content

Add a 2 hour offset to tests to avoid being a day short after the clocks change#243194

Merged
bpasero merged 2 commits intomicrosoft:mainfrom
a-stewart:fix-dst-test-failures
Mar 13, 2025
Merged

Add a 2 hour offset to tests to avoid being a day short after the clocks change#243194
bpasero merged 2 commits intomicrosoft:mainfrom
a-stewart:fix-dst-test-failures

Conversation

@a-stewart
Copy link
Contributor

Fixes #243192

After the clocks change, this time 5 days ago is only 4 days 23 hours ago, which evaluates to 4 days ago and the test fails.

This change adds a bit more margin for error, but instead testing this time (-2 hours) 5 days ago.

@connor4312 connor4312 enabled auto-merge (squash) March 11, 2025 16:22
@vs-code-engineering vs-code-engineering bot added this to the March 2025 milestone Mar 11, 2025
@a-stewart
Copy link
Contributor Author

@bpasero - GitHub is showing Community PR ApprovalsExpected — Waiting for status to be reported to me - it says I have the approval, but doesn't offer any way to merge?

@bpasero bpasero closed this Mar 13, 2025
auto-merge was automatically disabled March 13, 2025 19:02

Pull request was closed

@bpasero bpasero reopened this Mar 13, 2025
@bpasero bpasero enabled auto-merge (squash) March 13, 2025 19:02
@bpasero bpasero merged commit 7430313 into microsoft:main Mar 13, 2025
11 checks passed
@a-stewart a-stewart deleted the fix-dst-test-failures branch March 13, 2025 19:17
@a-stewart
Copy link
Contributor Author

Thanks!

@roblourens
Copy link
Member

roblourens commented Mar 15, 2025

FYI this leads to a 2 hour window in which the test fails :) #243619

@a-stewart
Copy link
Contributor Author

a-stewart commented Mar 15, 2025

Ah, the yesterday mechanism does something different to the daysAgo mechanism.

For days ago it uses the time since the date, divided by 84600000 seconds. I had tested this with -22 hours to be sure there would be no issue around midnight.

But yesterday and today are special cases which test against midnight.

The above PR looks like a good solution to me.

@roblourens
Copy link
Member

Yeah I just realized that daysAgo works that way. I don't like it, when I say 3 days ago I'm not taking the time of day into account. But not going to rewrite it now :)

@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Apr 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Date.fromNowByDay.daysAgo fails after timezone change when run in US timezone

4 participants