fix: DayOfWeek now returns random day #24
Conversation
Codecov Report
@@ Coverage Diff @@
## master #24 +/- ##
=======================================
Coverage 97.82% 97.82%
=======================================
Files 8 8
Lines 368 368
=======================================
Hits 360 360
Misses 4 4
Partials 4 4
Continue to review full report at Codecov.
|
| func TestDayOfWeekReturnsDifferentValues(t *testing.T) { | ||
| dayMap := make(map[string]struct{}) | ||
| iterations := 5 // sufficiently large to assure we don't randomly get the same value again | ||
| for i := 0; i < iterations; i++ { |
There was a problem hiding this comment.
The loop creates a mathematical set of all returned values. Each call should return a random value but, by pure randomness, a couple calls might return the same value. Hence the loop to call enough times to make sure there are different values returned.
| //t.Log(day) | ||
| dayMap[day] = struct{}{} | ||
| } | ||
| assert.True(t, len(dayMap) > 1) |
There was a problem hiding this comment.
If there is more than one value in the set, we know we have that the underlying function is not always returning the same value.
| } | ||
| assert.True(t, len(dayMap) > 1) | ||
| } | ||
|
|
There was a problem hiding this comment.
Question: If you uncomment the t.Log line you can see that different values get returned on subsequent calls. But if you rerun the test, you get the same sequence of 5 values repeated. Wondering if there is any easy way to randomize the starting value?
There was a problem hiding this comment.
Because you already using map, I think you can also utilize the map to avoid the same value repeated.
day := GetDateTimer().DayOfWeek()
if _, ok := dayMap[day]; ok {
i--
continue
}
dayMap[day] = struct{}{}
t.Log(day) // Will print random and different day 5 times. 
| "log" | ||
| "reflect" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/bxcodec/faker/support/slice" | ||
| "github.com/stretchr/testify/assert" |
There was a problem hiding this comment.
I know testify is really helpful. But I would recommend to not using it here. I'd prefer if we just using a simple assertion manually like other test-case. It's just to avoid any possible future conflict related to dependencies because we don't use any dependency management tools here (Dep, Glide etc).
| //t.Log(day) | ||
| dayMap[day] = struct{}{} | ||
| } | ||
| assert.True(t, len(dayMap) > 1) |
There was a problem hiding this comment.
Referring to my first comment about testify, could you please change this into manual assertion without using testify? :D
Thank you
bxcodec
changed the title
Previously using the
day_of_weektag, as inwould always return
Sunday. Tracked this down to the fact that the code needs to use constants from the reference time"Mon Jan 2 15:04:05 MST 2006"as described https://gobyexample.com/time-formatting-parsing. So the correct value to use isMonday.Also included in this PR one way to test that we are not always getting the same value returned from the function.