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

Added Expect #398

Merged
merged 1 commit into from
Feb 20, 2019
Merged

Added Expect #398

merged 1 commit into from
Feb 20, 2019

Conversation

dg
Copy link
Member

@dg dg commented Feb 17, 2019

  • new feature
  • BC break? no

This PR attempts to simplify the comparsion of structures that contain some variable data like this:

$actual = ['a' => md5($s), 'b' => new DateTime, 'c' => rand()];
$expected = ???
Assert::equal($expected, $actual);

I'd like make assertions that

  • $actual is array
  • it contains keys a, b, c
  • and first value is hexa string: Assert::type('string') + Assert::match('%h%', $value)
  • second value is object DateTime: Assert::type('DateTime', $value)
  • the third value is integer: Assert::type('int', $value)) and >= 0 (Assert::true($value >= 0)

This PR allows to describe it using Assert::equal() + language similar to how Assert is used. Only Assert:: is replaced with Expect::

$actual = ['a' => md5($s), 'b' => new DateTime, 'c' => rand()];
$expected = [
	'a' => Expect::type('string')->andMatch('%h%')),
	'b' => Expect::type(DateTime::class), 
	'c' => Expect::type('int')->and(function ($val) { return $val >= 0; }),
];
Assert::equal($expected, $actual);

In case of error it writes:

Failed: ['a' => 'd41d8cd98f00b204e9800998ecf8427e', 'b' => DateTime(2019-02-17 22:36:36 +0100)(#532c), ...] should be equal to
    ... ['a' => type(string),match(%h%), 'b' => type(DateTime), 'c' => type(int),custom()]

Class name Expect and methods are just working names. Is Assert::equal() good choice?

}


public function assert(callable $cb): self
Copy link
Contributor

Choose a reason for hiding this comment

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

must require Closure to avoid collision with __call

@JanTvrdik
Copy link
Contributor

Maybe call it Constraint or Rule? Mockery calls it Matcher.

@JanTvrdik
Copy link
Contributor

JanTvrdik commented Feb 18, 2019

I think that it's quite problematic that both __call and __callStatic accept the same method names, making it probably impossible to document with @method annotation.

@dg
Copy link
Member Author

dg commented Feb 18, 2019

Or what about Is for readability?

Is::notSame($expected)
Is::equal($expected)
Is::notEqual($expected)
Is::contains($needle) // hmmm 
Is::notContains($needle)
Is::nan()
Is::truthy()
Is::falsey()
Is::count(int $count)
Is::type($type)
Is::match(string $pattern) // hmmm.. add alias Is::pattern(), Assert::pattern() ?
Is::matchFile(string $file)

These ones will probably will never used:

Is::same($expected)
Is::true()
Is::false()
Is::null()

@dg
Copy link
Member Author

dg commented Feb 18, 2019

I think that it's quite problematic that both __call and __callStatic accept the same method name, making it probably impossible to document with @method annotation.

In fact, bigger problem is that Is::abc()::xyz() is perfectly valid, but means something different, Is::xyz().

@dg
Copy link
Member Author

dg commented Feb 18, 2019

So better will be this syntax Value::type('string')->and(Value::match('%h%')),

@dg dg force-pushed the pull-equal branch 4 times, most recently from e8f6a0e to dafa51d Compare February 18, 2019 12:58
@milo
Copy link
Member

milo commented Feb 18, 2019

👍 What about Expect class name?

@milo
Copy link
Member

milo commented Feb 18, 2019

@dg You mentioned ::pattern() It is related to #350. I'm planning to split Assert::match() to ::matchRe() or ::regexp() or something like this.

@milo
Copy link
Member

milo commented Feb 18, 2019

And what about implement all supported static Assert methods on Value class instead of __callStatic()? Stack trace will be cleaner.

@dg
Copy link
Member Author

dg commented Feb 18, 2019

'Expect' sound good.

And what about implement all supported static Assert methods on Value class instead of __callStatic()? Stack trace will be cleaner.

This is of course possible, but where do you see the stack trace?

@milo
Copy link
Member

milo commented Feb 19, 2019

This is of course possible, but where do you see the stack trace?

An imaginary stack trace when exception thrown somehow in Assert. Small detail only.

Nothing. As I would never said that :)

@dg dg force-pushed the pull-equal branch 2 times, most recently from a9f7ce3 to 5bd73bc Compare February 20, 2019 00:41
@dg dg merged commit 6c36b1f into nette:master Feb 20, 2019
@dg dg deleted the pull-equal branch February 20, 2019 01:24
@dg dg changed the title Better Assert::equal() WIP Added Expect Mar 11, 2019
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.

3 participants