Skip to content

Conversation

@jnizet
Copy link
Contributor

@jnizet jnizet commented Nov 22, 2025

The migration previously migrated jasmine.clock().mockDate() to vi.setSystemTime(), which does not compile because vi.setSystemTime() requires an argument. This commit fixes it by migrating jasmine.clock().mockDate() to vi.setSystemTime(new Date()).

PR Checklist

Please check to confirm your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

The migration previously migrated `jasmine.clock().mockDate()` to `vi.setSystemTime()`, which does not compile because `vi.setSystemTime()` requires an argument.
This commit fixes it by migrating `jasmine.clock().mockDate()` to `vi.setSystemTime(new Date())`.
Copy link
Contributor

@atscott atscott left a comment

Choose a reason for hiding this comment

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

Should this be new Date(0) so you get a consistent base time?

@jnizet
Copy link
Contributor Author

jnizet commented Nov 22, 2025

@atscott The documentation of Jasmine says that the default value is now.
So I chose to keep the same behavior in the migrated code.

@atscott
Copy link
Contributor

atscott commented Nov 22, 2025

Ah, yes. I guess any test which uses the default can’t actually depend on the date in any way except for how much time has passed (right?). Don’t know at that point if it’s better to do new Date(0) to signal that’s just a best practice to avoid an oopsie or to leave it as defaulting to now. Probably the latter I suppose if technically now could have been mocked beforehand.

@alan-agius4 alan-agius4 requested a review from clydin November 24, 2025 09:43
@alan-agius4 alan-agius4 added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Nov 24, 2025
@clydin clydin added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Nov 24, 2025
@clydin clydin merged commit 32da5a8 into angular:main Nov 24, 2025
38 checks passed
@clydin
Copy link
Member

clydin commented Nov 24, 2025

This PR was merged into the repository. The changes were merged into the following branches:

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: @schematics/angular target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants