Skip to content

Conversation

@TheSuperiorStanislav
Copy link

No description provided.

@TheSuperiorStanislav TheSuperiorStanislav force-pushed the make-default-color-as-random branch from f93e7dc to 17415eb Compare August 16, 2024 04:45
@TheSuperiorStanislav TheSuperiorStanislav marked this pull request as ready for review August 16, 2024 04:48
@TheSuperiorStanislav TheSuperiorStanislav force-pushed the make-default-color-as-random branch from 17415eb to 17faf9b Compare June 11, 2025 09:18
Copy link
Contributor

@kingbuzzman kingbuzzman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not in favor of this approach, as I generally prefer to avoid introducing additional uncertainty into our tests. If you'd like to do this locally and call it "RandomImageField" or "FuzzyImageField", that's totally fine on your end. However, I don't think it makes sense to add this to the main repository, as it would require everyone to be aware of it and potentially adjust their code to accommodate it.

@TheSuperiorStanislav
Copy link
Author

I'm not in favor of this approach, as I generally prefer to avoid introducing additional uncertainty into our tests. If you'd like to do this locally and call it "RandomImageField" or "FuzzyImageField", that's totally fine on your end. However, I don't think it makes sense to add this to the main repository, as it would require everyone to be aware of it and potentially adjust their code to accommodate it.

To be honest it's hard to imagine test breaking because of color change. Currently default it's not explict, if test relies on specific color, i think it should be specified in Factory definition. I mean faker text (or any other providers) generate random stuff each time, and if user needs specific stuff, they set it explicitly. So why image factory should be different?

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.

2 participants