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

Custom fakeable types #230

Merged
merged 15 commits into from Apr 4, 2023
Merged

Conversation

mgilbir
Copy link
Contributor

@mgilbir mgilbir commented Mar 27, 2023

There are occasions in which it is not possible to add field tags in a struct in order to use this library.

This PR defines an single method interface that can be implemented by a type.

When gofakeit.Struct() or gofakeit.Slice()` calls the method in the interface, it uses the returned value as the generated value for the type.

There are now three ways to control the generation: field tags, Fakeable interface implementation, and gofakeit defaults. That is also the precedence in which they are applied in the case of multiple options being available. The reason why the field tags are always preferred over the rest of the options is because they are explicit instructions at the struct where the generation happens.

Would you be open to merging this?

@brianvoe brianvoe changed the base branch from master to develop March 27, 2023 16:51
@brianvoe
Copy link
Owner

I think I like the idea. It makes sense to me. Looks like all tests are passing so it should effect the current working usages.

Were you able to run coverage? Just want to make sure we are fully testing things.

Let me pass this along to a few other developers and get everyone's thoughts.

@mgilbir
Copy link
Contributor Author

mgilbir commented Mar 27, 2023

Thanks for the quick response!

I checked the code coverage. It changed a bit because there are a few error handling code paths that have been added but are not tested.

I'm happy to add additional tests to cover those cases if you want.

Before:
ok github.com/brianvoe/gofakeit/v6 1.211s coverage: 95.7% of statements
ok github.com/brianvoe/gofakeit/v6/cmd/gofakeit 0.931s coverage: 95.7% of statements
ok github.com/brianvoe/gofakeit/v6/cmd/gofakeitserver 0.538s coverage: 69.1% of statements
ok github.com/brianvoe/gofakeit/v6/data 0.545s coverage: 100.0% of statements

After:
ok github.com/brianvoe/gofakeit/v6 1.050s coverage: 95.3% of statements
ok github.com/brianvoe/gofakeit/v6/cmd/gofakeit 0.401s coverage: 95.7% of statements
ok github.com/brianvoe/gofakeit/v6/cmd/gofakeitserver 0.830s coverage: 69.1% of statements
ok github.com/brianvoe/gofakeit/v6/data 0.680s coverage: 100.0% of statements

@brianvoe
Copy link
Owner

I think as long as the coverage is good around what you did you should be fine

@mgilbir mgilbir mentioned this pull request Mar 28, 2023
@borosr
Copy link
Contributor

borosr commented Mar 28, 2023

First of all, I really like the idea! Only one thing that we could talk about. How could the interface work with the faker object? So, for example, if I want to generate a fake first name through the interface, right now the only way I can do it, is by using the global faker object and there is no way I can use a local faker object.

type testStruct1 struct {
	B string `fake:"{firstname}"`
}

type strTyp string

type testStruct2 struct {
	B strTyp
}

func (t strTyp) Fake() interface{} {
	return FirstName()
}

func ExampleFake() {
	var t1 testStruct1
	var t2 testStruct1
	var t3 testStruct2
	var t4 testStruct2
	New(314).Struct(&t1)
	New(314).Struct(&t2)
	New(314).Struct(&t3)
	New(314).Struct(&t4)

	fmt.Printf("%#v\n", t1)
	fmt.Printf("%#v\n", t2)
	fmt.Printf("%#v\n", t3)
	fmt.Printf("%#v\n", t4)
	// Expected Output:
	// gofakeit.testStruct1{B:"Margarette"}
	// gofakeit.testStruct1{B:"Margarette"}
	// gofakeit.testStruct2{B:"Margarette"}
	// gofakeit.testStruct2{B:"Margarette"}
	
	// Actual Output:
	// gofakeit.testStruct1{B:"Margarette"}
	// gofakeit.testStruct1{B:"Margarette"}
	// gofakeit.testStruct2{B:"Two Hearted Ale"}
	// gofakeit.testStruct2{B:"Maudite"}
}

I don't know if it's possible, but it would be nice to pass a faker object to the Fake method, what do you think?
My solution proposal would look something like this:

type Fakeable interface {
    // Fake returns a fake value for the type.
    Fake(faker *Faker) interface{}
}

@mgilbir
Copy link
Contributor Author

mgilbir commented Mar 28, 2023

Interesting idea!

To make it work nicely I've refactored a bit struct.go to pass a *Faker object around instead of the *rand.Rand.

fakeable.go Outdated Show resolved Hide resolved

type CustomInt int

func (c CustomInt) Fake(faker *gofakeit.Faker) interface{} {
Copy link
Owner

Choose a reason for hiding this comment

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

Can you provide a couple of different ints, uints and floats to compare them a little more to a representation of what various ints, uints and floats would be?

}
}

func ExampleFakeable() {
Copy link
Owner

Choose a reason for hiding this comment

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

Just to help give the best examples of this feature as possible can you throw in a couple of other custom examples to showcase what others can do with simple custom variables as well as structs?

@brianvoe
Copy link
Owner

brianvoe commented Apr 3, 2023

Everything looks great! Nice job! I added just a few comments. Nothing crazy, just somethings to help elevate examples and usages. The only other thing I would ask is just to update the readme with a good example usage.

Ill try to get this in soon and will do a change log and small marketing for this release.

@mgilbir
Copy link
Contributor Author

mgilbir commented Apr 3, 2023

Thanks for the review! I'll get to it.

BTW I have a follow up PR after this one is merged. Just mentioning it in case you want to hold the release until you decide if that one is interesting too as it builds on this and the other PR that you just merged.

@brianvoe
Copy link
Owner

brianvoe commented Apr 3, 2023

ya we can get this merged in once your ready and ill wait for you to submit the other pr and we can go from there.

@brianvoe
Copy link
Owner

brianvoe commented Apr 4, 2023

Let me know if their is anything else you would like to add before I merge this.

@mgilbir
Copy link
Contributor Author

mgilbir commented Apr 4, 2023

If you are happy with these last changes, feel free to merge and I will submit the next PR.

@brianvoe
Copy link
Owner

brianvoe commented Apr 4, 2023

ok sounds good.

@brianvoe brianvoe merged commit bdec0d0 into brianvoe:develop Apr 4, 2023
12 checks passed
@mgilbir mgilbir deleted the custom-fakeable-types branch April 4, 2023 22:19
@brianvoe
Copy link
Owner

@brianvoe
Copy link
Owner

https://www.reddit.com/r/golang/comments/12mf47m/gofakeit_v6210_adds_fakeable_interface

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.

None yet

3 participants