8000 [DX] Add new methods to DomCrawler to simplify some functional tests · Issue #11470 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
8000

[DX] Add new methods to DomCrawler to simplify some functional tests #11470

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
Closed
javiereguiluz opened this issue Jul 25, 2014 · 8 comments
Closed

[DX] Add new methods to DomCrawler to simplify some functional tests #11470

javiereguiluz opened this issue Jul 25, 2014 · 8 comments
Labels
DomCrawler

Comments

@javiereguiluz
Copy link
Member

The problem

The whitespace inside the HTML contents makes some functional tests harder than
they should be. Consider that you want to check the contents the following
element:

<p>
    Lorem ipsum
</p>

This test will fail:

$this->assertEquals(
    'Lorem ipsum',
    $crawler->filter('p')->first()->text()
);

You can obviously apply the trim() function, but this solution feels
unnatural:

$this->assertEquals(
    'Lorem ipsum',
    trim($crawler->filter('p')->first()->text())
);

The problem is worse when dealing with spaces and newlines:

<p>
    Lorem ipsum
    dolor sit
    amet
</p>

This solution no longer works:

$this->assertEquals(
    'Lorem ipsum dolor sit amet',
    trim($crawler->filter('p')->first()->text())
);

The solution

DomCrawler component could add two new methods called trim() and flatten().
The first one is equivalent to the PHP's trim() function:

<p>
    Lorem ipsum
</p>
$this->assertEquals(
    'Lorem ipsum',
    $crawler->filter('p')->first()->text()->trim()
);

The second one would also remove new lines and trim two or more spaces between
lines:

<p>
    Lorem ipsum
    dolor sit
    amet
</p>
$this->assertEquals(
    'Lorem ipsum dolor sit amet',
    $crawler->filter('p')->first()->text()->flatten()
);

My questions: is it worth it adding these two helper methods? Should I instead
create better tests? Can you think of a better solution to solve this problem?
@javiereguiluz
Copy link
Member Author

This is just a ping on this uncommented issue.

Is this something that someone could find it useful? Would you solve this "problem" using a different and more elegant approach?

8000

@merk
Copy link
Contributor
merk commented Aug 4, 2014

For what its worth I normally use assertContains in a method where I'm checking for presence of a string. I've never found being explicit about exact markup to be very helpful given the markup can change (along with exact contents of markup).

@javiereguiluz
Copy link
Member Author

@merk thanks for your comment. I agree with you about not needing to validate the exact HTML markup. But my use case is a bit different. In the applications that we develop, HTML contents are always tabulated and broken into several lines to improve its readability:

<p>
    Lorem ipsum dolor sit amet,
    consectetur adipisicing elit,
    sed do eiusmod tempor incididunt.
</p>

As you can see, the <p> element doesn't include any HTML markup, as it's just raw text contents. The problem is that the white spaces and new lines overcomplicate functional tests.

@merk
Copy link
Contributor
merk commented Aug 4, 2014

My point is more that is unnecessary to test for the presence of the entire string in your <p> tag, some of my apps contain things like 'Save User' in their button text, but for testing I'd be inclined just to search for a button that contains 'Save', since button text may end up getting changed by designers or front end developers.

@javiereguiluz
Copy link
Member Author

@merk I see it. Let's wait a bit more to see if we receive any other opinion. Otherwise, I will close this issue. Thanks.

@ricardclau
Copy link
Contributor

+1 to @merk 's comments

@jakzal
Copy link
Contributor
jakzal commented Aug 4, 2014

very good points @merk 👍

@javiereguiluz
Copy link
Member Author

Closing, as everybody agrees on not needing these methods. Thanks for your comments!

@javiereguiluz javiereguiluz mentioned this issue Jan 16, 2017
Closed
@vlakoff vlakoff mentioned this issue Jul 22, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
DomCrawler
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@javiereguiluz @jakzal @merk @ricardclau
0