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 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

This comment has been minimized.

Copy link
@JanTvrdik

JanTvrdik Feb 18, 2019

Contributor

must require Closure to avoid collision with __call

@JanTvrdik

This comment has been minimized.

Copy link
Contributor

JanTvrdik commented Feb 18, 2019

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

@JanTvrdik

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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 dg:pull-equal branch 4 times, most recently from e8f6a0e to dafa51d Feb 18, 2019
@milo

This comment has been minimized.

Copy link
Member

milo commented Feb 18, 2019

👍 What about Expect class name?

@milo

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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 dg:pull-equal branch 2 times, most recently from a9f7ce3 to 5bd73bc Feb 19, 2019
@dg dg force-pushed the dg:pull-equal branch from 5bd73bc to be3513b Feb 20, 2019
@dg dg merged commit 6c36b1f into nette:master Feb 20, 2019
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@dg dg deleted the dg:pull-equal branch Feb 20, 2019
@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
Projects
None yet
3 participants
You can’t perform that action at this time.