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.

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

Improve parameter name for assertGreaterThan(), assertGreaterThanOrEqual(), assertLessThan(), and assertLessThanOrEqual() #6010

Closed
wants to merge 1 commit into from

Conversation

semkoolennl
Copy link

Resolves #5849

Improved the $expected parameter name for the following constraints:

  • assertGreaterThan()
  • assertGreaterThanOrEqual()
  • assertLessThan()
  • assertLessThanOrEqual()

I decided to use minimum and maximum as parameter names as these make the comparisons clear and intuitive.

@sebastianbergmann sebastianbergmann added the feature/assertion Issues related to assertions and expectations label Oct 23, 2024
Copy link

codecov bot commented Oct 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.73%. Comparing base (0562f48) to head (efcbdca).

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #6010   +/-   ##
=========================================
  Coverage     94.73%   94.73%           
  Complexity     6439     6439           
=========================================
  Files           697      697           
  Lines         20252    20252           
=========================================
  Hits          19185    19185           
  Misses         1067     1067           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sebastianbergmann sebastianbergmann changed the title Improved parameter names for asserts (lessThan & greaterThan) Improve parameter name for assertGreaterThan(), assertGreaterThanOrEqual(), assertLessThan(), and assertLessThanOrEqual() Oct 23, 2024
@sebastianbergmann sebastianbergmann added the type/enhancement A new idea that should be implemented label Oct 23, 2024
@sebastianbergmann
Copy link
Owner

Cherry-picked into 11.4 and merged to 11.5 and main from there. Thanks!

@NanoSector
Copy link

Just wanted to note that this broke some of our tests with a dependency update, as we used named parameters with these functions.

Not sure if that can be seen as a backwards compatibility break in a patch version.

@sebastianbergmann
Copy link
Owner

Not sure if that can be seen as a backwards compatibility break in a patch version.

With PHPUnit, this is never a backwards compatibility break. Every source file of PHPUnit has this at the top:

/**
 * @no-named-arguments Parameter names are not covered by the backward compatibility promise for PHPUnit
 */

See https://github.com/sebastianbergmann/phpunit/blob/11.4.3/src/Framework/Assert.php#L70-L72, for example and the Backward Compatibility page.

@NanoSector
Copy link

Ah apologies, I was looking for that mention/page in the documentation and could not find it, I hadn't looked in the code or on the main site.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/assertion Issues related to assertions and expectations type/enhancement A new idea that should be implemented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parameter name $expected does not make sense for assertLessThan() etc.
4 participants