Added delegates for modifying the JUnit testcase classname and name #186
Conversation
…ame-delegate Michaelwestbrook/modify name delegate
Fixed lint errors
|
Just checking in on this PR. Any chance of getting this in? |
|
Did you review and consider my comments from a few months back? I’m hesitant to add another way of modifying something that’s already exposed for user control. Am I missing something? I admit to not having read the code changes since 4 months ago when I made some comments, and I don’t see that those were responded to. |
|
Hmm, I'm not sure what comments you are referring to. Where were they made? |
| @@ -195,6 +197,12 @@ | |||
| if(options.systemOut && typeof options.systemOut !== "function") { | |||
| throw new Error('option "systemOut" must be a function'); | |||
| } | |||
| if (options.modifyClassName && typeof options.modifyClassName !== "function") { | |||
| throw new Error('option "modifyClassName" must be a function'); | |||
There was a problem hiding this comment.
What does modifyClassName provide that isn't already available through modifySuiteName? The primary difference appears to be that modifySuiteName receives the suite as input, whereas modifyClassName receives the spec—is that the important distinction? It gives me pause to have two separate options which both are ultimately meant for modifying the classname property in the XML document, but providing one of the options prevents the other from being useful.
Since modifySuiteName is already available and is used to modify the classname property—it is called from within getFullyQualifiedSuiteName—I'd prefer a solution that doesn't introduce new ways of modifying the same property. If you need the spec available to your function, I'd propose to change the interface of modifySuiteName to also receive the spec itself as the second parameter, thus retaining backwards compatibility but presumably also giving you what you're looking for with this new option.
|
I tried to use the GitHub pull request review feature, which maybe I didn't utilize correctly... it looks like it might have just posted the comments now, because I never pressed a "submit" button or something in the past. In case it didn't show up for you, I've attached a picture of the comments here too.
|
No description provided.