Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

fix: DayOfWeek now returns random day #24

Merged
merged 2 commits into from
Jul 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion datetime.go
Original file line number Diff line number Diff line change
Expand Up @@ -593,12 +593,14 @@ var timezones = []string{
"Asia/Yekaterinburg",
}

// These example values must use the reference time "Mon Jan 2 15:04:05 MST 2006"
// as described at https://gobyexample.com/time-formatting-parsing
const (
BaseDate = "2006-01-02"
Time = "15:04:05"
Month = "January"
Year = "2006"
Day = "Sunday"
Day = "Monday"
DayOfMonth = "_2"
TimePeriod = "PM"
)
Expand Down
15 changes: 14 additions & 1 deletion datetime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ package faker

import (
"fmt"
"github.com/bxcodec/faker/support/slice"
"log"
"reflect"
"testing"
"time"

"github.com/bxcodec/faker/support/slice"
"github.com/stretchr/testify/assert"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

)

func TestSetDateTimer(t *testing.T) {
Expand Down Expand Up @@ -82,6 +84,17 @@ func TestDayOfWeek(t *testing.T) {
}
}

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++ {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

day := GetDateTimer().DayOfWeek()
//t.Log(day)
dayMap[day] = struct{}{}
}
assert.True(t, len(dayMap) > 1)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Referring to my first comment about testify, could you please change this into manual assertion without using testify? :D

Thank you

}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Owner

@bxcodec bxcodec Jun 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. 

func TestDayOfMonth(t *testing.T) {
d := GetDateTimer()
_, err := time.Parse(DayOfMonth, d.DayOfMonth())
Expand Down