Skip to content

test: make current time testable #1363

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.

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

Conversation

Komyyy
Copy link

@Komyyy Komyyy commented Feb 17, 2025

This PR replaces DateTime.now with ZulipBinding.utcNow.
Also, this PR reduces some redundant arguments for current time; these arguments are not needed because we can use withClock for testing.

@Komyyy Komyyy changed the title test: testable DateTime.now test: testable version of DateTime.now Feb 17, 2025
@Komyyy Komyyy marked this pull request as ready for review February 17, 2025 11:27
@Komyyy Komyyy changed the title test: testable version of DateTime.now zulip_binding: testable version of DateTime.now Feb 18, 2025
@PIG208 PIG208 added this to the M6: Post-launch milestone Feb 18, 2025
@chrisbobbe
Copy link
Collaborator

Thanks! fa5dfd5 added ZulipBinding.utcNow; can we use that, or is there a need to add a new field?

@PIG208
Copy link
Member

PIG208 commented May 31, 2025

I believe this is just a timezone conversion away from what we have in main. The part about replacing DateTime.now with the ZulipBinding equivalent is a nice follow-up we mentioned in 0c03009.

@chrisbobbe
Copy link
Collaborator

chrisbobbe commented Jun 1, 2025

So it is something we want, separately from utcNow? If so, Komyyy let's put the two fields side-by-side and make their dartdocs more parallel/similar, with bidirectional "see also" notes that help the reader decide which one to use.

@Komyyy
Copy link
Author

Komyyy commented Jun 2, 2025

So it is something we want, separately from utcNow? If so, Komyyy let's put the two fields side-by-side and make their dartdocs more parallel/similar, with bidirectional "see also" notes that help the reader decide which one to use.

I wasn't aware that this function had been added.
The ZulipBinding.now function can be replaced with DateTime.toLocal and ZulipBinding.utcNow.
Therefore, I will update this PR to replace DateTime.now with ZulipBinding.utcNow.

@Komyyy Komyyy marked this pull request as draft June 2, 2025 07:22
@Komyyy Komyyy marked this pull request as ready for review June 2, 2025 08:02
@Komyyy Komyyy changed the title zulip_binding: testable version of DateTime.now test: make current time testable Jun 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants