-
-
Couldn't load subscription status.
- Fork 388
[LiveComponent] Add events assertions in InteractsWithLiveComponents
#2712
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
Conversation
c994d18 to
a43398b
Compare
bc67a41 to
3f1ce0c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor wordings and types, but otherwise that's fine, thanks :)
| foreach ($parameters as $key => $value) { | ||
| $this->assertSame($value, $eventData['data'][$key] ?? null, \sprintf('EventData (%s) is not valid', $key)); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will helps by displaying a nice diff between expected vs actual data:
| foreach ($parameters as $key => $value) { | |
| $this->assertSame($value, $eventData['data'][$key] ?? null, \sprintf('EventData (%s) is not valid', $key)); | |
| } | |
| $this->assertEquals(expectedEventData, $event['data'], sprintf('The expected event "%s" data does not match.', $eventName)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep but with the global assertEquals, we must pass all expectedEventDatas.
If the event contains ['a' => 'b', 'c' => 'd'];
And I expect only ['a' => 'b'];
It throw an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's true, but I think that's the desired behavior. Asserting the whole event data makes more sense to me, but I may be biaised and I understand that sometimes you expect to assert an array subset, but it must be explicit.
I've seen it tons of time on multiple projects, where sometimes you assert everything, or sometimes only a subset, and every time that's a mess because the method-name / PHPUnit's way of working does not help.
I can suggest you this, so everyone would be happy:
// classic way
$this->assertComponentEmitEvent($component, 'foo'); // do not care about event data
$this->assertComponentEmitEventWithData($component, 'foo', []); // assert whole array
$this->assertComponentEmitEventWithDataSubset($component, 'foo', []); // assert subset array
// or a bit exotic
$this->assertComponentEmitEvent($component, 'foo'); // do not care about event data
$this->assertComponentEmitEvent($component, 'foo')->withData([]); // assert whole array
$this->assertComponentEmitEvent($component, 'foo')->withDataSubset([]); // assert subset arrayWDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// or a bit exotic
$this->assertComponentEmitEvent($component, 'foo'); // do not care about event data
$this->assertComponentEmitEvent($component, 'foo')->withData([]); // assert whole array
$this->assertComponentEmitEvent($component, 'foo')->withDataSubset([]); // assert subset array
I personally LOVE this! 😍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the ->with implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An anonym class is enough or I have to separate it ?
The limit of the anonyme class is the dot block : I can not chained the ->with
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A separate concrete class please :)
32e10b9 to
2c5191f
Compare
203644b to
2a6206e
Compare
InteractsWithLiveComponents
2a6206e to
c159411
Compare
|
Rebased, squashed, some rewords, changelog updated and some documentation fixed 💅🏻 |
…Event()` to test events
c159411 to
edf1fc6
Compare
|
Thanks @Arkalo2. |
… InteractsWithLiveComponents (Develog) This PR was merged into the 2.x branch. Discussion ---------- [LiveComponent] Add dispatch browser event assertion in InteractsWithLiveComponents | Q | A | -------------- | --- | Bug fix? | no | New feature? | yes <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- if yes, also update UPGRADE-*.md and src/**/CHANGELOG.md --> | Documentation? | yes <!-- required for new features, or documentation updates --> | Issues | no <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead --> | License | MIT Next to this PR #2712 I wanted to add test assertion on dispatchBrowserEvent and not only on emited event. I reuse same logic as the `ComponentWithEmit.php`. ``` $this->assertComponentDispatchEvent($testComponent->render(), 'browserEvent') ->withPayload(['amount' => 2, 'unit' => 'kg']) ->withPayloadSubset(['amount' => 2]) ; ``` `$this->assertComponentNotDispatchEvent($testComponent->render(), 'otherBrowserEvent');` Commits ------- 5943c89 [LiveComponent] Add dispatch browser event test
… InteractsWithLiveComponents (Develog) This PR was merged into the 2.x branch. Discussion ---------- [LiveComponent] Add dispatch browser event assertion in InteractsWithLiveComponents | Q | A | -------------- | --- | Bug fix? | no | New feature? | yes <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- if yes, also update UPGRADE-*.md and src/**/CHANGELOG.md --> | Documentation? | yes <!-- required for new features, or documentation updates --> | Issues | no <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead --> | License | MIT Next to this PR symfony/ux#2712 I wanted to add test assertion on dispatchBrowserEvent and not only on emited event. I reuse same logic as the `ComponentWithEmit.php`. ``` $this->assertComponentDispatchEvent($testComponent->render(), 'browserEvent') ->withPayload(['amount' => 2, 'unit' => 'kg']) ->withPayloadSubset(['amount' => 2]) ; ``` `$this->assertComponentNotDispatchEvent($testComponent->render(), 'otherBrowserEvent');` Commits ------- 5943c89f4ea [LiveComponent] Add dispatch browser event test
This MR simplify liveComponent event testing :
Actually with the Test Helper, we can't simply test whether an event has been dispatched from a component.
This PR add 2 asserts :