I refactored a lot of examples. I can do more.

I had something similar before. I had a custom case for every enum (because reflect can't get the consts that are valid). I was having trouble with supporting nested structs, which I guess we do not need to support.

What do you do for things likeExpiredAt andCreatedAt though? Bothtime.Time{}, but one should be in the future, one should benow or in the past. So you either need struct tags, or do something with the field names.

Other fields which need custom handling off the top of my head:

• All enums
• RbacRoles is[]string{}
• Name is usually regex limited. We could just make allstring alpha only.
• AvatarURL is a string, should be a valid url
• []byte is sometimes json, sometimes a hash
• IPAddress probably handle custom

I also don't want truly random data. If you don not specifyRBACRoles, we should default to a member.Provisioner should beecho for tests. So the faker should accept some defaults instead of random in many cases no?

I guess in my mind we shouldn't handle these bespoke situations with a generator anyways, because they are bespoke, so why generalize it?

Pretty much all fakers allow domain knowledge in the generator.

• https://pkg.go.dev/github.com/icrowley/fake?utm_source=godoc#pkg-index
• https://github.com/bxcodec/faker/blob/a94c89da64ee65404c21aef71bd83f3f27c1b8b8/example_with_tags_test.go#L10-L68

The problem with truly random data, is that is usually isn't useful. If we go that route, it's more likefuzzing thanfaking. If that is what we want, I can support the reflect path better.

Just to put the differences in the switch statement.

case database.Workspace:workspace,err:=db.InsertWorkspace(ctx, database.InsertWorkspaceParams{ID:takeFirst(orig.ID,uuid.New()),OwnerID:takeFirst(orig.OwnerID,uuid.New()),CreatedAt:takeFirst(orig.CreatedAt,time.Now()),UpdatedAt:takeFirst(orig.UpdatedAt,time.Now()),OrganizationID:takeFirst(orig.OrganizationID,uuid.New()),TemplateID:takeFirst(orig.TemplateID,uuid.New()),Name:takeFirst(orig.Name,namesgenerator.GetRandomName(1)),AutostartSchedule:orig.AutostartSchedule,Ttl:orig.Ttl,})require.NoError(t,err,"insert workspace")returnworkspace

vs

case database.Workspace:// Reflect based random data seeded with the original data.value:=random(orig)workspace,err:=db.InsertWorkspace(ctx,value)require.NoError(t,err,"insert workspace")returnworkspace

I think too the reflect based method would be inherently more challenging to maintain (unless we use a library). I think the current code is pretty straight forward for a new person to look at.

Hmm, I see what you mean. I think what I'd prefer here is:

package databasegenfuncNew(db database.Store)*Generator {return&Generator{db}}typeGeneratorstruct {db database.Store}func (g*Generator)Workspace(workspace database.Workspace) database.Workspace {ifworkspace.RequiredField==nil {panic("asdf")  } }func (g*Generator)APIKey() (database.APIKey,string) {}

Kinda a combo of what you have now and something new. I don't think it should go indatabasefake, because it's not specific to that abstraction (if I understand correctly), and I want PSQL tests to benefit from this as well.

I liked the idea of removing state, so I updated the code to look like this:https://github.com/coder/coder/blob/stevenmasley/fake_data/coderd/database/databasefake/generator.go

You are right about not storing state in the generator imo. This change does remove the need for the switch, so I could do:

funcTemplate(t*testing.T,db database.Store,seed database.Template) database.Template {template,err:=db.InsertTemplate(context.Background(), database.InsertTemplateParams{ID:takeFirst(seed.ID,uuid.New()),CreatedAt:takeFirst(seed.CreatedAt,time.Now()),UpdatedAt:takeFirst(seed.UpdatedAt,time.Now()),OrganizationID:takeFirst(seed.OrganizationID,uuid.New()),Name:takeFirst(seed.Name,namesgenerator.GetRandomName(1)),Provisioner:takeFirst(seed.Provisioner,database.ProvisionerTypeEcho),ActiveVersionID:takeFirst(seed.ActiveVersionID,uuid.New()),Description:takeFirst(seed.Description,namesgenerator.GetRandomName(1)),DefaultTTL:takeFirst(seed.DefaultTTL,3600),CreatedBy:takeFirst(seed.CreatedBy,uuid.New()),Icon:takeFirst(seed.Icon,namesgenerator.GetRandomName(1)),UserACL:seed.UserACL,GroupACL:seed.GroupACL,DisplayName:takeFirst(seed.DisplayName,namesgenerator.GetRandomName(1)),AllowUserCancelWorkspaceJobs:takeFirst(seed.AllowUserCancelWorkspaceJobs,true),})require.NoError(t,err,"insert template")returntemplate}

We can add required fields, but at present no objects have any required fields. I put it in thedatabasefake package originally because for postgres you have the problem of foreign key constraints. I can let people use it for postgres though, just know the generator does not have the intelligence to know of dependencies.

Example of the new package:

No generics (except for take), no reflect. Actually pretty clean and similar to our current helper functions we write.

Agreed. I like that@Emyrk

We should move it out ofdatabasefake tho. I think it can be incoderd/database/databasegen

Agreed. This came out way cleaner. Will refactor the tests I changed to use this.

It also will work with postgres, the caller just needs to handle the constraints on their own.

I think you are right,dbgen it is, then I'll merge

We should probably renamedatabasefake todbfake then. Consistency is important, so I didn't want to break it just for this package.

@Emyrk thoughts on the above?

I likedbfake.

We havedbtestutil anddbtype already.