Skip to content
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

test: improve pummel/test-timers.js #35175

Merged
merged 1 commit into from Sep 15, 2020
Merged

Conversation

Trott
Copy link
Member

@Trott Trott commented Sep 13, 2020

  • use Date.now() instead of new Date() because only the timestamp is
    ever used, so we don't need the full Date object
  • use separate start times recorded for the two different test cases
  • improve assertion messages
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Sep 13, 2020
@Trott
Copy link
Member Author

Trott commented Sep 13, 2020

* use Date.now() instead of new Date() because only the timestamp is
  ever used, so we don't need the full Date object
* use separate start times recorded for the two different test cases
* improve assertion messages

PR-URL: nodejs#35175
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
@Trott
Copy link
Member Author

Trott commented Sep 15, 2020

Landed in ae257ca

@Trott Trott merged commit ae257ca into nodejs:master Sep 15, 2020
@Trott Trott deleted the pummel-timers branch September 15, 2020 17:52
ruyadorno pushed a commit that referenced this pull request Sep 21, 2020
* use Date.now() instead of new Date() because only the timestamp is
  ever used, so we don't need the full Date object
* use separate start times recorded for the two different test cases
* improve assertion messages

PR-URL: #35175
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
@ruyadorno ruyadorno mentioned this pull request Sep 21, 2020
4 tasks
addaleax pushed a commit that referenced this pull request Sep 22, 2020
* use Date.now() instead of new Date() because only the timestamp is
  ever used, so we don't need the full Date object
* use separate start times recorded for the two different test cases
* improve assertion messages

PR-URL: #35175
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
@codebytere codebytere mentioned this pull request Sep 28, 2020
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
* use Date.now() instead of new Date() because only the timestamp is
  ever used, so we don't need the full Date object
* use separate start times recorded for the two different test cases
* improve assertion messages

PR-URL: nodejs#35175
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants