-
Notifications
You must be signed in to change notification settings - Fork 642
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
Make keyed data accessible with ActiveFixture
#11445
Conversation
src/test/ActiveFixture.php
Outdated
@@ -63,7 +63,8 @@ public function load(): void | |||
throw new InvalidArgumentException('Unable to save fixture data'); | |||
} | |||
|
|||
$this->ids[] = $arInstance->id; | |||
$this->data[$key] = array_merge($row, $arInstance->toArray()); |
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.
Should this be just $this->data[$key] = $arInstance;
?
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.
Yes probably, I think I got a bit tunnel-visioned trying to keep it close to the base method in Yii's `ActiveFixture class: https://github.com/yiisoft/yii2/blob/master/framework/test/ActiveFixture.php#L86
Description
Craft's implementation of
ActiveFixture
overrides theload
method. In doing so it is not populating thedata
property during the process.This causes an issue as it means if you are using this class you are unable to retrieve the data from the fixture. Using
$this->tester->grabFixture('my-fixture', 'alias-key')
returnsnull
. This PR fixes that issue and will now return the data for that fixture.This all gives great flexibility when writing tests that use fixtures as you will be able to write assertions based on the actual data without having to do any extra queries.