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
chore(ci): re-enable tests on Windows #5637
Conversation
67d1727
to
220c4f1
Compare
This comment has been minimized.
This comment has been minimized.
d05639b
to
95536d8
Compare
c61b5ce
to
ceab809
Compare
This commit runs the unit tests on Windows. There are two tests failing on Windows that we skip. I spoke to Mathias B and we agreed to defer debugging this for now in favour of getting tests running on Windows. But we didn't want to ignore it forever, hence giving the test a date at which it will start to fail.
ceab809
to
33e083b
Compare
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.
LGTM with some final nitpicks
test/mocha-utils.js
Outdated
@@ -92,6 +93,16 @@ global.itFailsFirefox = (...args) => { | |||
return it(...args); | |||
}; | |||
|
|||
global.itFailsWindowsUntilDate = (date, ...args) => { | |||
const today = new Date(); |
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.
Since we only need the timestamp to compare:
const today = new Date(); | |
const today = Date.now; |
Since this is pretty short we don't really need the today
variable (better: now
?) anymore.
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 could make this function accept a string for the date
parameter, since all callers do new Date(string)
anyway.
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.
i.e. itFailsWindowsUntilDate('2020-06-01', blah)
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.
May do that in a follow up PR - I'm also tempted to tidy up the test helpers a bit. Right now we have itFailsFirefox
(with no date) and itFailsWindowsUntilDate
. I'd quite like a itFails({ product: 'firefox', date: '... })
or similar API rather than bespoke methods every time
Co-Authored-By: Mathias Bynens <mathias@qiwi.be>
Co-Authored-By: Mathias Bynens <mathias@qiwi.be>
Co-Authored-By: Mathias Bynens <mathias@qiwi.be>
No description provided.