Skip to content

Fix build failure with newest @types/mocha #21075

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

Merged
merged 1 commit into from
Jan 8, 2018
Merged

Fix build failure with newest @types/mocha #21075

merged 1 commit into from
Jan 8, 2018

Conversation

ghost
Copy link

@ghost ghost commented Jan 8, 2018

Looks like in the newest version of @types/mocha, this callback can give us a string.

@ghost ghost requested a review from weswigham January 8, 2018 21:11
@@ -55,7 +55,7 @@ namespace Harness.Parallel.Worker {
retries() { return this; },
slow() { return this; },
timeout(n) {
timeout = n;
timeout = n as number;
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be +n instead?

Copy link
Author

Choose a reason for hiding this comment

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

Depends on whether we believe it to be a number already, or believe that it actually could be a string and want to convert it to a number at runtime. @weswigham might know if the new type definition is actually accurate to our situation

@weswigham
Copy link
Member

Technically if it's a string we should parse it using similar logic to mocha, but we don't use the string version at any callsite (ofc, it typechecked before), so a cast is likely fine for now. Doing more would be pointless, since we don't use it.

@ghost
Copy link
Author

ghost commented Jan 8, 2018

@mhegazy @weswigham Good to go?

@mhegazy mhegazy merged commit 464df8f into master Jan 8, 2018
@mhegazy mhegazy deleted the mocha_timeout branch January 8, 2018 22:34
@mhegazy mhegazy mentioned this pull request Jan 9, 2018
5 tasks
@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants