Skip to content

Various new traits#5

Open
larowlan wants to merge 1 commit intomainfrom
various-new-traits
Open

Various new traits#5
larowlan wants to merge 1 commit intomainfrom
various-new-traits

Conversation

@larowlan
Copy link
Copy Markdown
Member

  • SpinTrait for retrying in a test
  • Extra random methods (url, email, location, address)
  • Constraint violation asserts

Copy link
Copy Markdown
Contributor

@mstrelan mstrelan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a few nits. Also the other existing traits have README entries. It's a hassle though, do we think it's worthwhile?

Comment on lines +15 to +24
/**
* Generates a random URL.
*
* @return string
* Random URL.
*/
protected function randomUrl(): string {
$random = new Random();
return \sprintf('https://%s.com/%s', $random->name(), $random->name());
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've always felt we should use example.com as the domain name. Turns out there is now a reserved .example TLD.

Suggested change
/**
* Generates a random URL.
*
* @return string
* Random URL.
*/
protected function randomUrl(): string {
$random = new Random();
return \sprintf('https://%s.com/%s', $random->name(), $random->name());
}
/**
* Generates a random URL.
*
* @return string
* Random URL.
*/
protected function randomUrl(): string {
$random = new Random();
return \sprintf('https://%s.example/%s', $random->name(), $random->name());
}

*/
protected function randomEmail(): string {
$random = new Random();
return \sprintf('%s@%s.com', $random->name(), $random->name());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return \sprintf('%s@%s.com', $random->name(), $random->name());
return \sprintf('%s@%s.example', $random->name(), $random->name());

protected function randomLatLonPair(): string {
$lon = $this->randomPoint(-180, 180);
$lat = $this->randomPoint(-84, 84);
return \sprintf('POINT(%s %s)', $lon, $lat);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: floats are not strings

Suggested change
return \sprintf('POINT(%s %s)', $lon, $lat);
return \sprintf('POINT(%.05n %.05n)', $lon, $lat);

Comment thread src/Traits/SpinTrait.php
Comment on lines +13 to +15
* Executes a callable until it returns TRUE.
*
* Executes executing a task until a condition is met.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Executes a callable until it returns TRUE.
*
* Executes executing a task until a condition is met.
* Executes a callable until it returns TRUE.

Comment thread src/Traits/SpinTrait.php
Comment on lines +19 to +30
* @param int $count
* (optional) Number of times to try, defaults to 10.
* @param bool $throw
* (optional) Throw, TRUE to throw if the condition is not met.
*
* @return bool
* TRUE if lambda evaluated true.
*
* @throws \Exception
* When the condition is not met.
*/
protected function spin(callable $lambda, $count = 10, $throw = TRUE): bool {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:

Suggested change
* @param int $count
* (optional) Number of times to try, defaults to 10.
* @param bool $throw
* (optional) Throw, TRUE to throw if the condition is not met.
*
* @return bool
* TRUE if lambda evaluated true.
*
* @throws \Exception
* When the condition is not met.
*/
protected function spin(callable $lambda, $count = 10, $throw = TRUE): bool {
* @param non-negative-int $count
* (optional) Number of times to try, defaults to 10.
* @param bool $throw
* (optional) Throw, TRUE to throw if the condition is not met.
*
* @return bool
* TRUE if lambda evaluated true.
*
* @throws \Exception
* When the condition is not met.
*/
protected function spin(callable $lambda, int $count = 10, bool $throw = TRUE): bool {

Comment thread src/Traits/SpinTrait.php
Comment on lines +38 to +40
catch (\Exception $e) {
// Do nothing.
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-capturing catch:

Suggested change
catch (\Exception $e) {
// Do nothing.
}
catch (\Exception) {
// Do nothing.
}

Comment thread src/Traits/SpinTrait.php
}
// Max reached.
if ($throw) {
throw new \Exception(\sprintf('Condition was not met after %d attempts', $count));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have a more specific exception, maybe \Behat\Mink\Exception\ExpectationException.

Comment on lines +46 to +53
/**
* Generates a random lat/lon point.
*/
private function randomPoint(int $min, int $max): float {
$number = \mt_rand($min, $max);
$decimals = \mt_rand(1, \pow(10, 5)) / \pow(10, 5);
return \round($number + $decimals, 5);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also require PHP 8.3 and use Randomizer::getFloat.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants