Tests panic with following command

[Dragomir-Ivanov]

go test ./... -count 10 -p 1 -run Test_BasicEndToEnd

{"@timestamp":"2019-11-24T13:45:40.554779999-08:00","@version":"1","level":"warning","message":"DEPRECATED: '-backend' is no longer supported. Please set the backends by explicit parameters"}
{"@timestamp":"2019-11-24T13:45:40.555117901-08:00","@version":"1","Backends":{"simple":{"bob":"secret"}},"CookieDomain":"","CookieExpiry":0,"CookieHTTPOnly":true,"CookieName":"jwt_token","CookieSecure":true,"GracePeriod":5000000000,"Host":"localhost","JwtAlgo":"HS512","JwtExpiry":86400000000000,"JwtRefreshes":0,"JwtSecret":"...","JwtSecretFile":"","LogLevel":"info","LoginPath":"/login","LogoutURL":"","Oauth":{},"Port":"3000","Redirect":true,"RedirectCheckReferer":true,"RedirectHostFile":"","RedirectQueryParameter":"backTo","SuccessURL":"/","Template":"","TextLogging":false,"UserEndpoint":"","UserEndpointTimeout":5000000000,"UserEndpointToken":"","UserFile":"","event":"start","level":"info","message":"starting application: loginsrv","type":"lifecycle"}
{"@timestamp":"2019-11-24T13:45:41.556961414-08:00","@version":"1","correlation_id":"Pusj3FC0EI","level":"info","message":"successfully authenticated","type":"application","username":"bob"}
{"@timestamp":"2019-11-24T13:45:41.557337806-08:00","@version":"1","User_Agent":"Go-http-client/1.1","correlation_id":"Pusj3FC0EI","duration":0,"host":"localhost:3000","level":"info","message":"200 -\u003ePOST /login","method":"POST","proto":"HTTP/1.1","remote_ip":"127.0.0.1","response_status":200,"type":"access","url":"/login"}
/tmp/go-build719816027/b001/loginsrv.test flag redefined: host
panic: /tmp/go-build719816027/b001/loginsrv.test flag redefined: host

goroutine 26 [running]:
flag.(*FlagSet).Var(0xc0000a6120, 0x99cf00, 0xc000089980, 0x8f37f3, 0x4, 0x8fdec0, 0x15)
    /usr/local/go/src/flag/flag.go:848 +0x4ae
flag.(*FlagSet).StringVar(...)
    /usr/local/go/src/flag/flag.go:751
github.com/tarent/loginsrv/login.(*Config).ConfigureFlagSet(0xc000089980, 0xc0000a6120)
    /home/drago/loginsrv/login/config.go:132 +0x71
github.com/tarent/loginsrv/login.readConfig(0xc0000a6120, 0xc0000a6c10, 0x5, 0x5, 0x0, 0x0, 0x0)
    /home/drago/loginsrv/login/config.go:212 +0x2bd
github.com/tarent/loginsrv/login.ReadConfig(...)
    /home/drago/loginsrv/login/config.go:202
github.com/tarent/loginsrv.main()
    /home/drago/loginsrv/main.go:23 +0x89
created by github.com/tarent/loginsrv.Test_BasicEndToEnd
    /home/drago/loginsrv/main_test.go:22 +0x157
FAIL    github.com/tarent/loginsrv  1.009s
ok      github.com/tarent/loginsrv/caddy    0.003s [no tests to run]
?       github.com/tarent/loginsrv/caddy/demo   [no test files]
ok      github.com/tarent/loginsrv/htpasswd 0.004s [no tests to run]
ok      github.com/tarent/loginsrv/httpupstream 0.003s [no tests to run]
ok      github.com/tarent/loginsrv/logging  0.002s [no tests to run]
ok      github.com/tarent/loginsrv/login    0.003s [no tests to run]
ok      github.com/tarent/loginsrv/model    0.004s [no tests to run]
ok      github.com/tarent/loginsrv/oauth2   0.002s [no tests to run]
ok      github.com/tarent/loginsrv/osiam    0.003s [no tests to run]
FAIL

[kernle32dll]

Can confirm the issue. Had these locally, too. Seems like some concurrency problem.

[Dragomir-Ivanov]

Hmm, I think there are no goroutines involved, but map iteration is non-deterministic, so If any piece of code relies on map iteration order, it can bomb like that.

[g-w]

The issue is that ReadConfig uses the global flag.CommandLine var and adds the flags each time called. This leads to the panic. See: https://github.com/tarent/loginsrv/blob/master/login/config.go#L202

[Dragomir-Ivanov]

@g-w But when I run go test -p 1 it still panics, although it should execute only one test at a time.

[g-w]

@Dragomir-Ivanov The issue is not related to parallelism. The issue is that if you read the config twice, all flags are added twice and newer versions of the flag package prevent this. This trigger the panic you've observed. I added a pr with a workaround for the issue.

[Dragomir-Ivanov]

@g-w Thanks! Apparently it panics only on test count > 1, so your patch fixes that.