chore(infrastructure): Better error messages in verifyDefaultAdapter #3516
Conversation
This reverts commit c04c3c5.
Codecov Report
@@ Coverage Diff @@
## master #3516 +/- ##
=========================================
Coverage ? 98.41%
=========================================
Files ? 120
Lines ? 5033
Branches ? 619
=========================================
Hits ? 4953
Misses ? 80
Partials ? 0Continue to review full report at Codecov.
|
This reverts commit 005caec.
acdvorak
changed the title
test/unit/helpers/foundation.js
Outdated
| const expectedSet = toSet(expectedArray); | ||
| const addedStr = getAddedStr(actualSet, expectedSet); | ||
| const removedStr = getRemovedStr(actualSet, expectedSet); | ||
| const messages = [addedStr, removedStr].filter((val) => Boolean(val)); |
There was a problem hiding this comment.
(val) => val.length > 0 ? Makes it a little more clear this is just filtering out empty strings...
test/unit/helpers/setup.js
Outdated
| export function setupFoundationTest(FoundationClass) { | ||
| // Make code coverage happy by instantiating the class with no arguments. |
There was a problem hiding this comment.
I'm confused... where were we missing coverage due to this? Judging by this comment it should currently be impossible for us to have 100% branch coverage on foundations, but that's clearly not the case...
There was a problem hiding this comment.
All I know is that Dialog code coverage was stuck at 13/14 branches until I added this line. 🤷♂️
There was a problem hiding this comment.
Without this line:
| File | Statements | Branches | Functions | Lines | |||||
|---|---|---|---|---|---|---|---|---|---|
| constants.js | 100% | 3/3 | 100% | 0/0 | 100% | 0/0 | 100% | 3/3 | |
| foundation.js | 100% | 64/64 | 92.86% | 13/14 | 100% | 22/22 | 100% | 61/61 | |
| index.js | 100% | 46/46 | 100% | 6/6 | 100% | 31/31 | 100% | 45/45 | |
| util.js | 100% | 6/6 | 100% | 2/2 | 100% | 4/4 | 100% | 5/5 |
With this line:
| File | Statements | Branches | Functions | Lines | |||||
|---|---|---|---|---|---|---|---|---|---|
| constants.js | 100% | 3/3 | 100% | 0/0 | 100% | 0/0 | 100% | 3/3 | |
| foundation.js | 100% | 64/64 | 100% | 14/14 | 100% | 22/22 | 100% | 61/61 | |
| index.js | 100% | 46/46 | 100% | 6/6 | 100% | 31/31 | 100% | 45/45 | |
| util.js | 100% | 6/6 | 100% | 2/2 | 100% | 4/4 | 100% | 5/5 |
There was a problem hiding this comment.
I just ran coverage with/without this line and it only affects branch coverage in dialog, line-ripple, and list. I'll see if I can figure out why, but I am inclined to leave it out of this PR.
There was a problem hiding this comment.
I figured out why 3 specific components have diffs with this line. It's because for some reason those components are using a default parameter in their foundation constructors, whereas all other foundations use an Object.assign within the constructor instead. I'm not sure what prompted this deviation, as the Object.assign pattern existed much earlier.
There was a problem hiding this comment.
#3522 removes the default parameters from line-ripple and list.
…aterial-components#3516) (cherry picked from commit 92fd9a7)
This PR adds more helpful error messages when adapters don't match the expected method names.
Before:
After: