Refactor _makeBadgeData -> coalesceBadge; test on [static dynamic endpoint] #2859
Conversation
…amic endpoint] `base.js` is pretty long and `_makeBadgeData` is one of the most complex functions in it. It's a pure function so it's easy to test in isolation. This moves the function into its own module and reorganizes the tests in a way that makes it evaluate what it's doing, and easier to test what is and isn't covered. Ref #2796 (comment)
paulmelnikow
changed the title
|
| ]) | ||
| }) | ||
|
|
||
| // This behavior isn't great and we might want to remove it. |
There was a problem hiding this comment.
What's the thinking on removing it? I actually just noticed this not too long ago when I forgot to set a default badge label and noticed the category being used in my local env. If we removed this, would there be an error thrown (programmer error) or what would the label show as alternatively?
There was a problem hiding this comment.
Throwing an error is a good idea.
There was a problem hiding this comment.
I think @chris48s was the first person who articulate this was a bad/unexpected use of the category.
|
This looks good to go IMO. I added a couple questions inline but basically ready to approve |
|
Oh and I was also wondering if this warranted a full service test run? (I see it was already run against a few) |
|
Could if you’d like! I think the coverage is there in “core” though I’ve zero objections if you’d like to run more tests. |
calebcartwright
changed the title
calebcartwright
changed the title
|
The tests that were failing (David, TeamCity, etc.) were the same as what's been failing in the daily run so no issues here |
base.jsis pretty long and_makeBadgeDatais one of the most complexfunctions in it.
It's a pure function so it's easy to test in isolation. This moves the function into its own module and reorganizes the tests in a way that makes it evaluate what it's doing, and easier to test what is and isn't covered.
Ref #2796 (comment)