-
Notifications
You must be signed in to change notification settings - Fork 201
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
Get wct-mocha compiling clean. #755
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,16 +48,16 @@ export interface ReporterFactory { | |
} | ||
|
||
interface ExtendedTest extends Mocha.ITest { | ||
err: {}; | ||
err: {}|undefined; | ||
} | ||
|
||
/** | ||
* A Mocha-like reporter that combines the output of multiple Mocha suites. | ||
*/ | ||
export default class MultiReporter implements Reporter { | ||
private readonly reporters: ReadonlyArray<Reporter>; | ||
private readonly parent: MultiReporter|undefined; | ||
private readonly basePath: string; | ||
readonly reporters: ReadonlyArray<Reporter>; | ||
readonly parent: MultiReporter|undefined|null; | ||
readonly basePath: string; | ||
total: number; | ||
private currentRunner: null|Mocha.IRunner; | ||
/** Arguments that would be called on emit(). */ | ||
|
@@ -73,7 +73,7 @@ export default class MultiReporter implements Reporter { | |
*/ | ||
constructor( | ||
numSuites: number, reporters: ReporterFactory[], | ||
parent: MultiReporter|undefined) { | ||
parent: MultiReporter|undefined|null) { | ||
this.reporters = reporters.map((reporter) => { | ||
return new reporter(this); | ||
}); | ||
|
@@ -92,12 +92,10 @@ export default class MultiReporter implements Reporter { | |
} | ||
|
||
/** | ||
* @param location The location this reporter represents. | ||
* @return A reporter-like "class" for each child suite | ||
* that should be passed to `mocha.run`. | ||
*/ | ||
childReporter(location: Location|string): ReporterFactory { | ||
const name = this.suiteTitle(location); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this was straight up not used? Huh. Good find. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just turned on warning for unused vars and props. That's also why I made some properties not private, because they weren't used anywhere, but I wasn't confident enough to delete them, so they're public for now. |
||
childReporter(): ReporterFactory { | ||
// The reporter is used as a constructor, so we can't depend on `this` being | ||
// properly bound. | ||
const self = this; | ||
|
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 am learning some TypeScript from this PR.
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.
Yeah, I'd prefer to rewrite these deep merge functions (we have two!) into something less magical.. but that's for another time