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

[6.x] Fix undefined property in WithFaker #31083

Merged
merged 1 commit into from
Jan 9, 2020
Merged

Conversation

driesvints
Copy link
Member

#30992 introduced a side effect where resolving faker would fail if the trait was implemented on a class that hasn't set the framework as a property.

Fixes #31071

@crynobone
Copy link
Member

crynobone commented Jan 9, 2020

Does this fully solve the problem?

If $this->app doesn't exist then config() helper usage should actually failed as well.

In 7.x we could split the basic usage of WithFaker to illuminate/testing and only resolves via $this->app for Illuminate\Foundation\Testing\WithFaker.

@GrahamCampbell GrahamCampbell deleted the fix-with-faker-trait branch January 9, 2020 14:57
@driesvints
Copy link
Member Author

@crynobone no, the framework can still be initialized so the config helper can still work. The problem was that if the trait was used on a class that doesn't has an "app" property that it would give a notice about the non-existing property.

@telkins
Copy link
Contributor

telkins commented Jan 10, 2020

FYI...it's also a problem if it's used in a "normal" test class, but in a data provider method.

@driesvints
Copy link
Member Author

@telkins if it's a different problem then this then please open an issue and provide steps to reproduce it, thanks.

@telkins
Copy link
Contributor

telkins commented Jan 10, 2020

And...it looks like this change addresses the issue. (I just updated locally to quickly test to make sure that it works...and it does.)

So...thanks...! :-)

@telkins
Copy link
Contributor

telkins commented Jan 10, 2020

@telkins if it's a different problem then this then please open an issue and provide steps to reproduce it, thanks.

It's exactly the same....I just wanted to confirm that the fix resolved it. I've had issue with data providers in general (because the app isn't up/available) and faker, too...for obvious reasons.

Again...thanks...! :-)

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.

WithFaker references $this->app which might be missing
4 participants