slug
slug copied to clipboard
gosimple
Make safe for use with concurrent goroutines (fix race hazards)
Hello and thank for this changes.
I moved removing of Go 1.13 requirement to another PR and merged with master #42 because I always squash pull requests and wanted it in separate commit. Thank you for showing me this.
Travis run tests by default with -race https://github.com/gosimple/slug/blob/master/.travis.yml#L18
So if you add test it should find if anything bad happen to the code at future.
Please, add test that will catch race condition if some change break this code. Travis run tests by default with -race
The -race flag only detects races encountered by the test code that's run. The way the current tests are written doesn't trigger races.
The original API for the module is built around modifying globals, so naturally there is, and always will be, a race hazard using that API. That's why I've added a new API.
The old API is now implemented in terms of the new one, so testing the old is also testing the new.
In 05c89ad I've added t.Run and t.Parallel calls to demonstrate the problem. I've left the t.Parallel calls commented out. If they are uncommented then the tests with fail. Use go test -race -p N ... to change the level of parallelism.
Also, re the go 1.13 change, I did that wrong. I should have changed it to go 1.12 or go 1.11 rather than removing it.