Update harness to use integrated vdom from widget-core #70
Conversation
Codecov Report
@@ Coverage Diff @@
## master #70 +/- ##
==========================================
- Coverage 99.08% 98.56% -0.52%
==========================================
Files 10 10
Lines 653 626 -27
Branches 160 163 +3
==========================================
- Hits 647 617 -30
- Misses 0 1 +1
- Partials 6 8 +2
Continue to review full report at Codecov.
|
|
The difference in code coverage is a red herring. Because there was quite a bit of code stripped because of leveraging the projector mixin from widget core, it is causing the code coverage to be 😭 |
kitsonk
changed the title
[ci skip]
src/support/assertRender.ts
Outdated
| } | ||
|
|
||
| if (Array.isArray(actual) && Array.isArray(expected)) { | ||
| assertChildren(actual, expected); | ||
| } | ||
| else if (Array.isArray(actual) || Array.isArray(expected)) { | ||
| throwAssertionError(actual, expected, message); | ||
| throwAssertionError(actual, expected, Array.isArray(actual) ? `Expected "${expected === null ? 'null' : typeof expected}" but got an array` : `Expected an array but got "${actual === null ? 'null' : typeof actual}"`, message); |
There was a problem hiding this comment.
I refactored all the messages, you are right it was messy and unreadable.
src/harness.ts
Outdated
| if (vnode && vnode.domNode) { | ||
| target = vnode.domNode as Element; | ||
| const dnode = findDNodeByKey(this._widgetHarness.lastRender, key); | ||
| if (dnode && !isWNode(dnode)) { |
There was a problem hiding this comment.
any reason why not isHNode instead of !isWNode?
There was a problem hiding this comment.
Not that I can think of... Wasn't thinking.
|
The coverage is a bit of a red herring, as it is more related to the changes in widget-core causing some less branch coverage, plus the overall reduction of code because of the removal of the widget.classes logic. |
|
I accidentally committed some change to the diagnostics of assert render that I had in another branch which I was going to submit after this PR, but they leaked in, so I merged the rest of them. Overall it just improves the information you get from assert render when something goes wrong, instead of scratching your head, so hopefully we can turn a blind eye. |
Type: feature
The following has been addressed in the PR:
Description:
This PR updates the harness to be compatible with the integrated vdom from widget-core (PR dojo/widget-core#728.
It also makes a few other changes, some of them breaking:
Uses the Projector in a sandbox mode, therefore widgets are rendered on a document fragment instead of actually in the DOM. This is more consistent with the concepts of harnessing and unit testing.Actually that was causing issues on Safari I couldn't fully resolve. Basically Safari 9 doesn't like dispatching events on DOM owned by a document fragment..setPropertiesand.setChildrenwill now properly type guard. This is a breaking change in that many cases people might have had to specify two generic arguments to have the harness work.This will need to be updated once the upstream PR is merged and published.