-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Allowed named classes and functions as BlockNames #5154
Changes from all commits
2ec0c8b
79a11aa
b335f41
db2fc7d
0bae63c
67dcfe6
8cd8505
74ca575
c4b8343
e1f6c7f
9c6b2c5
0744cf9
e547ef4
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 |
---|---|---|
|
@@ -38,7 +38,7 @@ import expectationResultFactory from '../expectation_result_factory'; | |
export default function Suite(attrs: Object) { | ||
this.id = attrs.id; | ||
this.parentSuite = attrs.parentSuite; | ||
this.description = attrs.description; | ||
this.description = convertDescriptorToString(attrs.description); | ||
this.throwOnExpectationFailure = !!attrs.throwOnExpectationFailure; | ||
|
||
this.beforeFns = []; | ||
|
@@ -181,6 +181,33 @@ Suite.prototype.addExpectationResult = function() { | |
} | ||
}; | ||
|
||
function convertDescriptorToString(descriptor) { | ||
if ( | ||
typeof descriptor === 'string' || | ||
typeof descriptor === 'number' || | ||
descriptor === undefined | ||
) { | ||
return descriptor; | ||
} | ||
|
||
if (typeof descriptor !== 'function') { | ||
throw new Error('describe expects a class, function, number, or string.'); | ||
} | ||
|
||
if (descriptor.name !== undefined) { | ||
return descriptor.name; | ||
} | ||
|
||
const stringified = descriptor.toString(); | ||
const typeDescriptorMatch = stringified.match(/class|function/); | ||
const indexOfNameSpace = | ||
typeDescriptorMatch.index + typeDescriptorMatch[0].length; | ||
const indexOfNameAfterSpace = stringified.search(/\(|\{/, indexOfNameSpace); | ||
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. @JoshuaKGoldberg I'm looking at converting this to TS, and it's not happy about the second argument here. It's also not documented at MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/search Should I just remove it? 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. @SimenB ha, sure. Looks like yet another "bug" fixed by TypeScript! 😍 |
||
const name = stringified.substring(indexOfNameSpace, indexOfNameAfterSpace); | ||
|
||
return name.trim(); | ||
} | ||
|
||
function isAfterAll(children) { | ||
return children && children[0].result.status; | ||
} | ||
|
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.
If something silly, such as a number or
{ toString() { return "nope"; }
is passed here, it'll crash. How would you like this function to deal with those error handling cases?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.
Can you just make the function throw in that case and ask people to provide a string, class or function instead?