Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug: assertSee() can not assert title tag. #3984

Closed
kenjis opened this issue Dec 13, 2020 · 10 comments · Fixed by #4070
Closed

Bug: assertSee() can not assert title tag. #3984

kenjis opened this issue Dec 13, 2020 · 10 comments · Fixed by #4070
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@kenjis
Copy link
Member

kenjis commented Dec 13, 2020

Describe the bug
assertSee() can not assert title tag.

$result->assertSee('<title>CodeIgniter Tutorial</title>'); -> Pass
$result->assertSee('CodeIgniter Tutorial', 'title'); -> Fail

CodeIgniter 4 version
4.0.4

Affected module(s)
Tests

Expected behavior, and steps to reproduce if appropriate
assertSee('CodeIgniter Tutorial', 'title'); should pass when the page has the title tag.

Context

  • OS: [macOS 10.15.7]
  • Web server [n/a]
  • PHP version [7.4.13]
@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Dec 13, 2020
@paulbalandan
Copy link
Member

I'm not familiar with DomXPath it uses under the hood, but it seems when the 1st arg of assertSee has tags, it considers the whole DOM. However, removing the tags, the code will search only within the <body/>. I dunno if that is a bug for you or is desirable.

@kenjis
Copy link
Member Author

kenjis commented Jan 6, 2021

I don't know why it searches only within the <body/>.

@michalsn
Copy link
Member

michalsn commented Jan 6, 2021

The first example that passes uses simple mb_strpos() for search. But the second one that fails uses the DOMXPath class, which has buggy rules that force a search only inside the body tag.

@michalsn michalsn mentioned this issue Jan 6, 2021
Merged
5 tasks
@sfadschm
Copy link
Contributor

sfadschm commented Jan 6, 2021

@paulbalandan is right here.
When using assertSee, the see method of DOMParser is called. If no element selector is passed to assertSee, this searches the whole DOM. If an element is passed, the DOMParser->doXPath function is used for searching. In this function body implicitly always gets added to the search string.
I feel this is more a consistency issue than a bug. Taking the UG literally, it

 asserts that text/HTML is on the page

so I would think the <title> tag in the <head> should never be found. This is also what the PHPDoc states:

Assert that the desired text can be found in the result body.

However, DOMParser->see and doXPath do not explicitly state that only the body is searched, but rather the whole DOM.

My personal opinion:
I do not see, why doXPath should only search the DOM of body. Btw. doXPath even behaves differently when using different kinds of selectors (e.g., passing a tag with an id will only search the body, but passing an id without a tag will search the whole DOM).

Options that I see:
1 modify the DOMParser->see() method to always use doXPath
2 modify doXPath to search the whole DOM anytime and update the assertSee UG
3 modify DOMParser and doXPath by adding an additional optional parameter bool $onlyBody = false/true to enable switching

@sfadschm
Copy link
Contributor

sfadschm commented Jan 6, 2021

The first example that passes uses simple mb_strpos() for search. But the second one that fails uses the DOMXPath class, which has buggy rules that force a search only inside the body tag.

:D three minutes too late.
Can you also update the assertSee PHPDoc? It now searches the whole result, not only the result body.

@michalsn
Copy link
Member

michalsn commented Jan 6, 2021

😅

I'm not a linguistic expert but

Assert that the desired text can be found in the result body.

doesn't sound to me like the suggestion of a body tag... but I might be wrong. English is not my first language, so can I hear one more opinion on this one before I push a change?

@sfadschm
Copy link
Contributor

sfadschm commented Jan 6, 2021

😅

I'm not a linguistic expert but

Assert that the desired text can be found in the result body.

doesn't sound to me like the suggestion of a body tag... but I might be wrong. English is not my first language, so can I hear one more opinion on this one before I push a change?

No worries, neither am I. Just the way I read those docs. I'm in for other opinions :)

@kenjis
Copy link
Member Author

kenjis commented Jan 6, 2021

My English is not good. :-)

The User Guide says:

You can perform tests to see if specific elements/text/etc exist with the body of the response with the following assertions.

I read it as the body of the response object, or the HTTP response body (not in the response header).
Not in the body tag.

@sfadschm
Copy link
Contributor

sfadschm commented Jan 6, 2021

Okay, just leave it then :-)

@sfsf9797
Copy link

sfsf9797 commented Mar 5, 2021

Hi, do you mind share how you get $result?
thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix DOMParser rules to search also outside the body tag michalsn/CodeIgniter4
5 participants
@kenjis @michalsn @sfsf9797 @sfadschm @paulbalandan