8000 Added Expect by dg · Pull Request #398 · nette/tester · GitHub
[go: up one dir, main page]
Skip to content

Added Expect#398

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

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

Conversation

@dg
Copy link
Member
@dg dg commented Feb 17, 2019 •
edited
Loading
  • 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?

JanTvrdik
JanTvrdik reviewed Feb 18, 2019
View reviewed changes
src/Framework/Value.php Outdated
}


public function assert(callable $cb): self
Copy link
Contributor
@JanTvrdik JanTvrdik Feb 18, 2019

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
JanTvrdik commented Feb 18, 2019

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

@JanTvrdik
Copy link
Contributor
JanTvrdik commented Feb 18, 2019 •
edited
Loading

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 •
edited
Loading

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 •
edited
Loading

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 •
edited
Loading

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 added a commit that referenced this pull request Feb 20, 2019
@dg
added Expect for complex assertions formulation (#398)
1ffb7b5
@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

Reviewers

1 more reviewer

@JanTvrdik JanTvrdik JanTvrdik left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dg @JanTvrdik @milo
0