cweill / gotests

Automatically generate Go test boilerplate from your source code.
Apache License 2.0
4.92k stars 346 forks source link

Having tests as map vs slice #148

Closed butuzov closed 3 years ago

butuzov commented 3 years ago

I wonder if having maps instead of the slice is desired feature for gotests users. I made a simple implementation based on default and testify templates.

Can be checked with next parameters:

gotests -nosubtests -asmap -i   -only ^Func$  /path/demo.go
gotests -asmap  -only  ^Func$  /path/demo.go
gotests -parallel -only  ^Func$  /path/demo.go
gotests -only ^Func$  /path/demo.go

Benefits:

Cons:

cweill commented 3 years ago

@butuzov This seems like a good idea. Do you mind posting an example output test here as a comment? I'm curious what it would look like, and what the generated map key names would look like.

butuzov commented 3 years ago
// gotests -asmap -only ^Add$ /Users/butuzov/github/gotests/demo.go
func TestAddSimple(t *testing.T) {
    type args struct {
        a int
        b int
    }
    tests := map[string]struct {
        args args
        want int
    }{
        "Zero":              {args: args{a: 1, b: -1}, want: 0},
        "Zero (comutative)": {args: args{a: -1, b: 1}, want: 0},
    }
    for name, tt := range tests {
        t.Run(name, func(t *testing.T) {
            if got := Add(tt.args.a, tt.args.b); got != tt.want {
                t.Errorf("Add() = %v, want %v", got, tt.want)
            }
        })
    }
}

//  gotests -asmap -parallel -only ^Add$ /Users/butuzov/github/gotests/demo.go
func TestAddParallel(t *testing.T) {
    type args struct {
        a int
        b int
    }
    tests := map[string]struct {
        args args
        want int
    }{
        "Zero":              {args: args{a: 1, b: -1}, want: 0},
        "Zero (comutative)": {args: args{a: -1, b: 1}, want: 0},
    }
    for name, tt := range tests {
        tt := tt
        name := name
        t.Run(name, func(t *testing.T) {
            t.Parallel()
            if got := Add(tt.args.a, tt.args.b); got != tt.want {
                t.Errorf("Add() = %v, want %v", got, tt.want)
            }
        })
    }
}

// gotests -asmap -parallel -nosubtests -only ^Add$ /Users/butuzov/github/gotests/demo.go
func TestAddNoSubTest(t *testing.T) {
    type args struct {
        a int
        b int
    }
    tests := map[string]struct {
        args args
        want int
    }{
        "Zero":              {args: args{a: 1, b: -1}, want: 0},
        "Zero (comutative)": {args: args{a: -1, b: 1}, want: 0},
    }
    for name, tt := range tests {
        if got := Add(tt.args.a, tt.args.b); got != tt.want {
            t.Errorf("%q. Add() = %v, want %v", name, got, tt.want)
        }
    }
}

With a tons of tests, it looks like this - https://github.com/butuzov/gotests/blob/as_map/gotests_test.go, it's not really cute, but randomness in tests are quite useful.

butuzov commented 3 years ago

@cweill does it still looks like a good idea?

cweill commented 3 years ago

@butuzov Looks like a good idea to me, as it's similar to what Python's absl package uses [1]. Maybe a flag -named or -named_tests to make it more clear. Your PR should only need one or two unit tests, ideally. How does that sound?

[1] https://github.com/abseil/abseil-py/blob/master/absl/testing/parameterized.py#L80

butuzov commented 3 years ago

I like this idea, a bit different from --randomly-dont-reorganize I used in pytest, I need to bring it to my py projects as well.

butuzov commented 3 years ago

@cweill OK, so I did a thing(s).

There are several PRs from my side (I tried to granulate it as much as possible), this is an order we need to merge it in:

1) https://github.com/cweill/gotests/pull/150: Fixes CI, adds Subtests, fixes bug #136 in one of the tests 2) https://github.com/cweill/gotests/pull/151: Fixes another issue, you can run into while running tests (previous tmpls isn't cleaned). (this is one of two reasons why #152 is failing) 3) #152 introduces -named argument that adds functionality described above in this issue.

After every merge I suggest updating or branches and rerun tests.

butuzov commented 3 years ago

Did some tests with mergings. You might want to merge PR one by one (each day). So I can update the next one with and resolve conflicts (there are quite few conflicts.)

cweill commented 3 years ago

Thanks @butuzov ! Let's try to get these in this week.

I approved and merged #150.

Please update #151 to HEAD and I will have another look after you resolve my comments.