assertQueryContentContains searching through all nodes found #5212

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
@EpeeCheeze
Contributor

EpeeCheeze commented Oct 2, 2013

When content is not in the first node of the document, assertQueryContentContains does not find it. This change checks all the nodes for the content when using the AbstractHttpControllerTestCase.

@@ -330,25 +330,23 @@ public function testAssertXpathQueryCountMax()
public function testAssertQueryContentContains()
{
$this->dispatch('/tests');
- $this->assertQueryContentContains('div#content', 'foo');

This comment has been minimized.

@Maks3w

Maks3w Oct 4, 2013

Member

Instead change this test case please add a new one

@Maks3w

Maks3w Oct 4, 2013

Member

Instead change this test case please add a new one

+ $path, $match
+ ));
+ }
+ $currentValue = $node->nodeValue;

This comment has been minimized.

@Maks3w

Maks3w Oct 4, 2013

Member

This could be outside of the loop. By this way only at finish the var is assigned.

@Maks3w

Maks3w Oct 4, 2013

Member

This could be outside of the loop. By this way only at finish the var is assigned.

- $this->assertEquals($result->current()->nodeValue, $match);
+ throw new PHPUnit_Framework_ExpectationFailedException(sprintf(
+ 'Failed asserting node denoted by %s CONTAINS content "%s"',
+ $path, $match

This comment has been minimized.

@Maks3w

Maks3w Oct 4, 2013

Member

Each argument must go in his own line (PSR-2)

@Maks3w

Maks3w Oct 4, 2013

Member

Each argument must go in his own line (PSR-2)

+ if ($node->nodeValue == $match) {
+ throw new PHPUnit_Framework_ExpectationFailedException(sprintf(
+ 'Failed asserting node DENOTED BY %s DOES NOT CONTAIN content "%s"',
+ $path, $match

This comment has been minimized.

@Maks3w

Maks3w Oct 4, 2013

Member

Same here

@Maks3w

Maks3w Oct 4, 2013

Member

Same here

@Maks3w

This comment has been minimized.

Show comment
Hide comment
@Maks3w

Maks3w Oct 4, 2013

Member

I think that there are two scenarios here.

  • At least one result have content
  • All results must have content

Could you elaborate this a bit more for cover both scenarios?

Thank you

Member

Maks3w commented Oct 4, 2013

I think that there are two scenarios here.

  • At least one result have content
  • All results must have content

Could you elaborate this a bit more for cover both scenarios?

Thank you

@EpeeCheeze

This comment has been minimized.

Show comment
Hide comment
@EpeeCheeze

EpeeCheeze Oct 16, 2013

Contributor

The documentation states that this query looks for at least one of the nodes contains the element. This is also the same functionality that was in ZF1.

Contributor

EpeeCheeze commented Oct 16, 2013

The documentation states that this query looks for at least one of the nodes contains the element. This is also the same functionality that was in ZF1.

weierophinney added a commit that referenced this pull request Oct 23, 2013

Merge pull request #5212 from EpeeCheeze/master
assertQueryContentContains searching through all nodes found

weierophinney added a commit that referenced this pull request Oct 23, 2013

[#5212] incorporate feedback
- Incorporated feedback from @Maks3w
- ensured all multi-line sprint() calls had one argument per line

weierophinney added a commit that referenced this pull request Oct 23, 2013

@ghost ghost assigned weierophinney Oct 23, 2013

weierophinney added a commit to zendframework/zend-test that referenced this pull request May 15, 2015

Merge pull request zendframework/zendframework#5212 from EpeeCheeze/m…
…aster

assertQueryContentContains searching through all nodes found

weierophinney added a commit to zendframework/zend-test that referenced this pull request May 15, 2015

[zendframework/zendframework#5212] incorporate feedback
- Incorporated feedback from @Maks3w
- ensured all multi-line sprint() calls had one argument per line

weierophinney added a commit to zendframework/zend-test that referenced this pull request May 15, 2015

weierophinney added a commit to zendframework/zend-test that referenced this pull request May 15, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment