Convert children arrays to DOM specs prior to performing comparisons.

[alexjeffburke]

As of this commit the full DOMNode to spec conversion is run for all
children specified as strings which allows ignore syntax to work.

Fixes #217.

[alexjeffburke]

Just a quick note that I've also protected the possibility of specifying an invalid HTML string within the children array - for the comparison to work correctly, there must only be a single node. In the case of getting this wrong, the code will output that it found a NodeList where only a single node is allowed.

[coveralls]

Pull Request Test Coverage Report for Build 813

• 12 of 13 (92.31%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.1%) to 91.703%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
lib/index.js	12	13	92.31%

Totals
Change from base Build 808:	0.1%
Covered Lines:	533
Relevant Lines:	563

---

💛 - Coveralls

[papandreou]

This is a breaking change that we'll need to discuss. Before a string passed in the to satisfy spec was matched against the text content when the LHS contained a text node. With this change it'll be interpreted as an HTML string.

In other words, this test used to pass, but won't with this change:

it('should succeed with a complex child', function() {
  body.innerHTML = '<div>&lt;div foo="bar"&gt;hey&lt;/div&gt;</div>';
  expect(body.firstChild, 'to satisfy', {
    children: ['<div foo="bar">hey</div>']
  });
});

I agree that it would be useful to be able specify the requirements for a subtree as a compact HTML string, but I think we need to come up with another syntax for it.

What about something like:

expect(element, 'to satisfy', {
  children: [ { outerHTML: '<div foo="bar">hey</div>'} ]
})

[papandreou]

Why shouldn't this work the same for XML? (Also, the var childNodes below has a !isHtml case?)

If there's a good reason to have this difference I'd rather that the comment stated it rather than what the code obviously does :)

[alexjeffburke]

@papandreou that's a good point, I'll tweak the comment.

Basically, without this one of the added tests (which was actually a port of the examples) seemed to cause the XML parser to break. IIRC the specific case was you go to compare an XML node like <hello>string</hello> against a spec like { children: ['string'] }. What the code will try to do is parse 'hello', which in HTML will produce a text node and all is well but I'm not sure that works in HTML.

[alexjeffburke]

@papandreou ahh so it was always the text content but only if it was a string?

That's interesting - being unaware of that, and given that strings are converted to DOMNodes in many other places I had just assumed that children were always a deeper comparison. Which they are (if I understand this correctly) in the case of specifying an object or a dom node or an object spec.

I know that plain strings were almost always treated as nodes elsewhere, so I sort of get the consistency but it still feels odd. But under these constraints this does need more thought. I guess the issue came up when the text could suddenly contain something like an ignore comment.

@papandreou thinking aloud, unexpected-reaction supplies ignore as an actual DOMCommentNode. So, what I could do is remove converting text entirely from this change but keep the DOM passthrough working right - this would maintain the plain string invariant and then we could use your outerHTML idea. Does that sound workable?

[alexjeffburke]

Oh, one other thought - outerHTML works pretty well in HTML mode, but would we need another alias to make it feel at home when dealing with XML?

[papandreou]

The current to satisfy logic is that how strings are interpreted is dependent on whether the LHS is a text node or an element. So this actually passes:

it('should succeed with a complex child', function() {
  body.innerHTML =
    '<div>&lt;abc&gt;<div foo="bar">hey</div>&lt;def&gt;</div>';
  expect(body.firstChild, 'to satisfy', {
    children: [
      '<abc>',
      '<div foo="bar">hey</div>',
      '<def>'
    ]
  });
});

This is symmetric with how the same specs work when matching a text node or an element at the "top level", so in that sense it's consistent.

It's bad news for matching the <!--ignore--> comment against a text node, of course. Maybe we can come up with something else for that, maybe based on expect.it?

[alexjeffburke]

@papandreou yeah I agree. I hadn't fully grasped the backwards compatibility issues and it wasn't intended to be a change that had the broad consequences it ended up having. What I'd suggest is taking a peek at #219 instead.

I believe it is doesn't break things while still supporting the actual case in unexpected-reaction that I was attempting to fix. I'd prefer to get that in and then follow-up with something more suitable for this case like the outerHTML suggestion you made :)

[sunesimonsen]

#219 seems more approachable.