Skip to content

Added Expect - #398

Merged
dg merged 1 commit into
nette:masterfrom
dg:pull-equal
Feb 20, 2019
Merged

Added Expect#398
dg merged 1 commit into
nette:masterfrom
dg:pull-equal

Conversation

@dg

@dg dg commented Feb 17, 2019

Copy link
Copy Markdown
Member
  • 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?

Comment thread src/Framework/Value.php Outdated
}


public function assert(callable $cb): self

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.

must require Closure to avoid collision with __call

@JanTvrdik

Copy link
Copy Markdown
Contributor

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

@JanTvrdik

JanTvrdik commented Feb 18, 2019

Copy link
Copy Markdown
Contributor

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

dg commented Feb 18, 2019

Copy link
Copy Markdown
Member Author

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

dg commented Feb 18, 2019

Copy link
Copy Markdown
Member Author

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

dg commented Feb 18, 2019

Copy link
Copy Markdown
Member Author

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

milo commented Feb 18, 2019

Copy link
Copy Markdown
Member

👍 What about Expect class name?

@milo

milo commented Feb 18, 2019

Copy link
Copy Markdown
Member

@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

milo commented Feb 18, 2019

Copy link
Copy Markdown
Member

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

@dg

dg commented Feb 18, 2019

Copy link
Copy Markdown
Member Author

'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

milo commented Feb 19, 2019

Copy link
Copy Markdown
Member

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