Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

feat: Add ability to ignore interface{} population with SetIgnoreInterface() #123

Merged
merged 4 commits into from
Jan 25, 2021

Conversation

nashikb
Copy link
Contributor

@nashikb nashikb commented Oct 28, 2020

This PR adds a flag that tells faker to ignore the population of interface{} types.

This is useful when populating complex structures for Marshal testing, where you don't have the control or ability to use a non-interface{}. This makes testing complex structure marshaling a LOT simpler, since otherwise you would have to faker every element in the structure above and beside an interface{}.

The default behavior is to not ignore interface{}, so no behavior changes should be seen.

A test has been added.

Thanks!

@codecov
Copy link

codecov bot commented Oct 28, 2020

Codecov Report

Merging #123 (533c450) into master (02fc1b2) will decrease coverage by 1.69%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #123      +/-   ##
==========================================
- Coverage   93.87%   92.17%   -1.70%     
==========================================
  Files          11       11              
  Lines        1289     1086     -203     
==========================================
- Hits         1210     1001     -209     
- Misses         43       46       +3     
- Partials       36       39       +3     
Impacted Files Coverage Δ
faker.go 90.27% <100.00%> (-0.98%) ⬇️
internet.go 76.69% <0.00%> (-6.64%) ⬇️
uuid.go 82.35% <0.00%> (-3.37%) ⬇️
datetime.go 97.46% <0.00%> (-0.80%) ⬇️
lorem.go 100.00% <0.00%> (ø)
phone.go 100.00% <0.00%> (ø)
price.go 100.00% <0.00%> (ø)
person.go 100.00% <0.00%> (ø)
address.go 100.00% <0.00%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 02fc1b2...533c450. Read the comment docs.

…e of numbers. Good for reliable testing of 0 values.
@bxcodec
Copy link
Owner

bxcodec commented Jan 21, 2021

Hi @nashikb

Thanks for the PR, I was so busy lately, haven't seen this repo for a while.
Is using ignore tag doesn't help?

type Person struct {
  Metadata interface{} `faker:"-"`
}

@nashikb
Copy link
Contributor Author

nashikb commented Jan 21, 2021

Hi @nashikb

Thanks for the PR, I was so busy lately, haven't seen this repo for a while.
Is using ignore tag doesn't help?

type Person struct {
  Metadata interface{} `faker:"-"`
}

This works for struct declarations that I have control over. But for my use case, I'm populating some structs in external libraries over which I have no control. The inability to apply faker attributes to external libraries is still not possible, but that's an issue that can be worked around. But hitting a single interface{} is fatal to the faker, with no recourse.

It could be worth at least considering ignoring interface{}s as the default.

@bxcodec
Copy link
Owner

bxcodec commented Jan 24, 2021

I see that makes sense.

I wonder if we can add a test-case but using 3rd party library that contains the interface as you explained. Just to making sure.

@bxcodec
Copy link
Owner

bxcodec commented Jan 24, 2021

I think, let's merge this.
After it merged, would you mind trying if it's working as you describe above?

@nashikb
Copy link
Contributor Author

nashikb commented Jan 24, 2021

Of course. We've been using this tweak in our tests since October '20 with no issues, so I'm pretty confident a merge-down will work just fine.

As for an external use case, that does make sense; but my concern was adding a dependency on faker tests that could silently break. Also, using a struct without faker:"-"on an interface{} seems to me to perform the same validation but without the dependency.

Thanks!

Copy link
Owner

@bxcodec bxcodec left a comment

Choose a reason for hiding this comment

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

Alright, thanks for the PR

@bxcodec bxcodec merged commit 74585ae into bxcodec:master Jan 25, 2021
@bxcodec
Copy link
Owner

bxcodec commented Jan 25, 2021

@nashikb
I'm planning to release it soon this week, in the meantime, you can download it from the master branch. 🙏
Just in case there's a bug, we can solve it quickly

@nashikb
Copy link
Contributor Author

nashikb commented Jan 25, 2021

Running my tests with GO111MODULE=off works fine off of master (hitting master via go.mod and the v3 path in the go.mod is tough).

My tests use 100s of repeated randomizations of structs, so I'm pretty confident nothing's amiss. (famous last words)

Once there's a release I'll immediately test again and delete my fork.

Thanks!

@nashikb
Copy link
Contributor Author

nashikb commented Feb 4, 2021

How's the release going? :-)

@bxcodec
Copy link
Owner

bxcodec commented Feb 5, 2021

https://github.com/bxcodec/faker/releases/tag/v3.6.0

released here.

Anyway, if you have time, mind adding some docs or examples? It's okay if you're busy. I'll try to look up on it this weekend

@nashikb
Copy link
Contributor Author

nashikb commented Feb 5, 2021

I've switched over and all my tests are passing, so it looks good to me. I'll look at a quick drawup on the reasoning and examples.

@nashikb
Copy link
Contributor Author

nashikb commented Mar 3, 2021

After putting together this very artificial example, I realize that the testing harnesses I'm working with are fairly esoteric and particular to conversions to JSON wrappers like grpc-gateway.

This is an example testify test that shows how ignoring interfaces can be useful for faking and comparing partial structures net interface{}es.


import (
	"encoding/json"
	"reflect"
	"testing"

	"github.com/bxcodec/faker/v3"
	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
	"github.com/stretchr/testify/suite"
)

type FancyType string

func NewFancyType(s string) FancyType {
	return FancyType(s)
}

func (ft FancyType) String() string {
	return string(ft)
}

type PreValue struct {
	Name  string    `json:"name"`
	Data  FancyType `json:"data"`
	Count int32     `json:"count,omitempty"`
}

type PostValue struct {
	Name   string      `json:"name"`
	Data   string      `json:"data"`
	Count  int64       `json:"count,omitempty"`
	Unused interface{} `json:"unused,omitempty"`
}

func ConvertPreToPost(pre PreValue) PostValue {
	return PostValue{
		Name:  pre.Name,
		Data:  pre.Data.String(),
		Count: int64(pre.Count),
	}
}

// Compare compares two arbitrary interfaces.
func RequireEqual(require *require.Assertions, a, b interface{}) {
	aBlob, err := json.MarshalIndent(a, "", "  ")
	require.Nil(err)
	bBlob, err := json.MarshalIndent(b, "", "  ")
	require.Nil(err)
	require.EqualValues(string(aBlob), string(bBlob))
}

func TestExampleTestSuite(t *testing.T) {
	FakerSetup()
	suite.Run(t, new(ExampleTestSuite))
}

// FakerSetup sets up Faker settings and custom type Providers.
func FakerSetup() {
	// Ignore interface{} population so that comparisons can be made to a subset of the structure.
	faker.SetIgnoreInterface(true)

	_ = faker.AddProvider("FancyType", func(v reflect.Value) (interface{}, error) {
		var value string
		if err := faker.FakeData(&value); err != nil {
			panic(err)
		}
		return NewFancyType(value), nil
	})
}

type ExampleTestSuite struct {
	suite.Suite
	//
	assert  *assert.Assertions
	reps    int
	require *require.Assertions
	t       *testing.T
}

// SetupTest initializes each test's state.
func (ts *ExampleTestSuite) SetupTest() {
	ts.assert = ts.Assert()
	ts.reps = 20
	ts.require = ts.Require()
	ts.t = ts.T()
	_ = faker.SetRandomMapAndSliceSize(15)
	_ = faker.SetRandomNumberBoundaries(0, 2^32-1)
}

func (ts *ExampleTestSuite) TestEntityConversion() {
	for testRep := 0; testRep < ts.reps; testRep++ {
		preValue := PreValue{}
		err := faker.FakeData(&preValue)
		ts.require.Nil(err)

		postValue := ConvertPreToPost(preValue)
		ts.require.Nil(err)
		ts.require.NotNil(postValue)

		RequireEqual(ts.require, preValue, postValue)
	}
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants