-
Notifications
You must be signed in to change notification settings - Fork 16
adds missing clock properties #51
base: master
Are you sure you want to change the base?
Conversation
@@ -0,0 +1,8 @@ | |||
import { LibraryTypeInstance, LibraryMethodInstance, LibraryPropertyInstance, LibraryEventInstance } from "../libraries"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we adding this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It slipped through my notice. I'll amend my commit.
@@ -11,7 +11,149 @@ x = Clock.Time`); | |||
engine.execute(ExecutionMode.RunToEnd); | |||
|
|||
const value = engine.memory.values["x"]; | |||
expect(value.toValueString()).toMatch(/[0-9]{2}:[0-9]{2}:[0-9]{2}/); | |||
expect(value.toValueString()).toMatch(/[0-9]{2}:[0-9]{2}:[0-9]{2} A|PM/); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears that adding the AM/PM
made the tests fail. Why did we do that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strange. On the contrary, tests were failing on my system without it. Maybe it depends on the locale? In that case we should manually construct the string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can easily reproduce this, just try:
LANG=en_US node -e 'console.log((new Date()).toLocaleString())'
028f56c
to
fa5b649
Compare
have a look at the test regex. Would it better to get current time values and then compare?