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

Assert ignoring line ending #4641

Closed
samdark opened this issue Apr 4, 2021 · 4 comments
Closed

Assert ignoring line ending #4641

samdark opened this issue Apr 4, 2021 · 4 comments
Labels
feature/assertion Issues related to assertions and expectations type/enhancement A new idea that should be implemented

Comments

@samdark
Copy link

samdark commented Apr 4, 2021

In many cases line ending difference doesn't matter at all but the test will fail in many cases because of it. For example:

  1. When checked out from git, user may have settings to convert line endings.
  2. Line ending differences when running on other OS vs what's used in tests.

We use the following assertion-methods: https://github.com/yiisoft/yii2/blob/master/tests/TestCase.php#L117

That would be great to have line-ending insensitive assertions out of the box.
@samdark samdark added the type/enhancement A new idea that should be implemented label Apr 4, 2021
@samdark
Copy link
Author

samdark commented Apr 6, 2021

I can do a pull request if the idea is alright.

@samdark samdark mentioned this issue Apr 15, 2021
Closed
@vjik vjik mentioned this issue May 18, 2021
Closed
@sebastianbergmann
Copy link
Owner

I could have use this just now for 9504d0d ;-)

@jrfnl
Copy link
Contributor

jrfnl commented Jan 3, 2022 •
edited
Loading

Hiya!

I've just been looking at the implementation of this and it raises some questions with me regarding the assertStringEqualsStringIgnoringLineEndings implementation:

  1. The term Equal (as in assertStringEqualsStringIgnoringLineEndings) is generally used for loose type comparisons in PHPUnit, but is used here for a type safe comparison.
  2. The actual comparison done is (mostly) type safe (Same) and would be so even when strict_types would be turned off or if the type declarations in the method would be changed to mixed, due to strtr() being used, which always returns a string.

So, my questions are:

  • Is assertStringEqualsStringIgnoringLineEndings() the correct name/assertion to be added ?
    Or should there be an assertEqualsIgnoreLineEndings() and an assertSameIgnoreLineEndings() ?
    This also implies that the normalizeLineEndings() method should allow for receiving mixed type, do a type check and would only normalize if the received type is actually a string (and return the original value unchanged if it isn't).
  • And if the new assertions would be transformed like this, what about also recursively handling string values in arrays to allow for a comparison of arrays ignoring line endings ?

I'd be happy to attempt a PR for this, but would like your opinion about this first.

@jrfnl
Copy link
Contributor

jrfnl commented Jan 3, 2022

Oh and another question: should there be Not versions of these assertions available ?

@bbatsche bbatsche mentioned this issue Mar 10, 2022
Closed
@jrfnl jrfnl mentioned this issue Mar 19, 2022
Closed
@sebastianbergmann sebastianbergmann added the feature/assertion Issues related to assertions and expectations label Sep 23, 2022
jrfnl added a commit to Yoast/PHPUnit-Polyfills that referenced this issue Mar 30, 2023
@jrfnl @vjik @sebastianbergmann
PHPUnit 10 | AssertIgnoringLineEndings trait: polyfill the Assert::as…
fa21946 
…sertStringEqualsStringIgnoringLineEndings() et al methods

PHPUnit 10.0.0 introduces the new `Assert::assertStringEqualsStringIgnoringLineEndings()` and `Assert::assertStringContainsStringIgnoringLineEndings()` methods.

This commit:
* Adds two traits with the same name.
    One to polyfill the methods when not available in PHPUnit.
    The other - an empty trait - to allow for `use`-ing the trait in PHPUnit versions in which the methods are already natively available.
* Logic to the custom autoloader which will load the correct trait depending on the PHPUnit version used.
* An availability test and limited functional test for the functionality polyfilled.

Note: the function name for the `private` `normalizeLineEndings()` method is a little convoluted - `normalizeLineEndingsForIgnoringLineEndingsAssertions()`.
This is intentional to prevent potential naming collisions with pre-existing end-user defined methods for the same, which may exist in other traits used in tests, which would be hard to solve due to the method only existing in the non-empty trait.

Includes:
* Adding the new polyfill to the existing `TestCases` classes.

Refs:
* sebastianbergmann/phpunit#4641
* sebastianbergmann/phpunit#4670 (and follow up commits)
* sebastianbergmann/phpunit#5279

Co-authored-by: Sergei Predvoditelev <sergey.predvoditelev@gmail.com>
Co-authored-by: Sebastian Bergmann <sb@sebastian-bergmann.de>
jrfnl added a commit to Yoast/PHPUnit-Polyfills that referenced this issue Mar 30, 2023
@jrfnl @vjik @sebastianbergmann
PHPUnit 10 | AssertIgnoringLineEndings trait: polyfill the Assert::as…
f4b0421 
…sertStringEqualsStringIgnoringLineEndings() et al methods

PHPUnit 10.0.0 introduces the new `Assert::assertStringEqualsStringIgnoringLineEndings()` and `Assert::assertStringContainsStringIgnoringLineEndings()` methods.

This commit:
* Adds two traits with the same name.
    One to polyfill the methods when not available in PHPUnit.
    The other - an empty trait - to allow for `use`-ing the trait in PHPUnit versions in which the methods are already natively available.
* Logic to the custom autoloader which will load the correct trait depending on the PHPUnit version used.
* An availability test and limited functional test for the functionality polyfilled.

Note: the function name for the `private` `normalizeLineEndings()` method is a little convoluted - `normalizeLineEndingsForIgnoringLineEndingsAssertions()`.
This is intentional to prevent potential naming collisions with pre-existing end-user defined methods for the same, which may exist in other traits used in tests, which would be hard to solve due to the method only existing in the non-empty trait.

Includes:
* Adding the new polyfill to the existing `TestCases` classes.

Refs:
* sebastianbergmann/phpunit#4641
* sebastianbergmann/phpunit#4670 (and follow up commits)
* sebastianbergmann/phpunit#5279

Co-authored-by: Sergei Predvoditelev <sergey.predvoditelev@gmail.com>
Co-authored-by: Sebastian Bergmann <sb@sebastian-bergmann.de>
jrfnl added a commit to Yoast/PHPUnit-Polyfills that referenced this issue Mar 30, 2023
@jrfnl @vjik @sebastianbergmann
PHPUnit 10 | AssertIgnoringLineEndings trait: polyfill the Assert::as…
8fc29f0 
…sertStringEqualsStringIgnoringLineEndings() et al methods

PHPUnit 10.0.0 introduces the new `Assert::assertStringEqualsStringIgnoringLineEndings()` and `Assert::assertStringContainsStringIgnoringLineEndings()` methods.

This commit:
* Adds two traits with the same name.
    One to polyfill the methods when not available in PHPUnit.
    The other - an empty trait - to allow for `use`-ing the trait in PHPUnit versions in which the methods are already natively available.
* Logic to the custom autoloader which will load the correct trait depending on the PHPUnit version used.
* An availability test and limited functional test for the functionality polyfilled.

Note: the function name for the `private` `normalizeLineEndings()` method is a little convoluted - `normalizeLineEndingsForIgnoringLineEndingsAssertions()`.
This is intentional to prevent potential naming collisions with pre-existing end-user defined methods for the same, which may exist in other traits used in tests, which would be hard to solve due to the method only existing in the non-empty trait.

Includes:
* Adding the new polyfill to the existing `TestCases` classes.

Refs:
* sebastianbergmann/phpunit#4641
* sebastianbergmann/phpunit#4670 (and follow up commits)
* sebastianbergmann/phpunit#5279

Co-authored-by: Sergei Predvoditelev <sergey.predvoditelev@gmail.com>
Co-authored-by: Sebastian Bergmann <sb@sebastian-bergmann.de>
jrfnl added a commit to Yoast/PHPUnit-Polyfills that referenced this issue Apr 1, 2023
@jrfnl @vjik @sebastianbergmann
PHPUnit 10 | AssertIgnoringLineEndings trait: polyfill the Assert::as…
bf4f816 
…sertStringEqualsStringIgnoringLineEndings() et al methods

PHPUnit 10.0.0 introduces the new `Assert::assertStringEqualsStringIgnoringLineEndings()` and `Assert::assertStringContainsStringIgnoringLineEndings()` methods.

This commit:
* Adds two traits with the same name.
    One to polyfill the methods when not available in PHPUnit.
    The other - an empty trait - to allow for `use`-ing the trait in PHPUnit versions in which the methods are already natively available.
* Logic to the custom autoloader which will load the correct trait depending on the PHPUnit version used.
* An availability test and limited functional test for the functionality polyfilled.

Note: the function name for the `private` `normalizeLineEndings()` method is a little convoluted - `normalizeLineEndingsForIgnoringLineEndingsAssertions()`.
This is intentional to prevent potential naming collisions with pre-existing end-user defined methods for the same, which may exist in other traits used in tests, which would be hard to solve due to the method only existing in the non-empty trait.

Includes:
* Adding the new polyfill to the existing `TestCases` classes.

Refs:
* sebastianbergmann/phpunit#4641
* sebastianbergmann/phpunit#4670 (and follow up commits)
* sebastianbergmann/phpunit#5279

Co-authored-by: Sergei Predvoditelev <sergey.predvoditelev@gmail.com>
Co-authored-by: Sebastian Bergmann <sb@sebastian-bergmann.de>
@jrfnl jrfnl mentioned this issue Apr 1, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature/assertion Issues related to assertions and expectations type/enhancement A new idea that should be implemented
Projects
None yet
Milestone
No milestone
3 participants
@sebastianbergmann @samdark @jrfnl