-
-
Notifications
You must be signed in to change notification settings - Fork 59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
person.go: 100% test coverage #33
person.go: 100% test coverage #33
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work here, I just have added some comments, my main question is about the RandomGenerator
type, today we already have the NewWithSeed
method that could be used for situations where you need "expected" values, maybe instead of adding a new code here you can use the existing one.
I totally missed the |
@ab22 no problem, I really appreciate your time helping to do the best here, please let me know if you have any question or need some help with some part of the code 👍 |
30ff5c2
to
b4e466d
Compare
Build has been in queue for a bit and status hasn't changed. Edit: forced pushed a commit to re-trigger. Edit2: build is not starting for some reason. CC @jaswdr |
b4e466d
to
bfdcc7a
Compare
@@ -0,0 +1,26 @@ | |||
package test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove this file, the Faker
structure should be enough to fake the seed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that I have to use Faker.NewWithSource
but how will I be able to control the returned number from the rand.Source
without a dummy implementation like this?
I apologize if I'm not following. I think might be missing something from the implementation of rand.Source
so if you could provide a small sample on how to control the returned numbers it would be great. @jaswdr
@@ -0,0 +1,17 @@ | |||
package test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove this file, the Faker
structure should be enough to fake the seed, if you need you can add tests to the faker_test.go
file
@@ -3,6 +3,8 @@ package faker | |||
import ( | |||
"strings" | |||
"testing" | |||
|
|||
"github.com/jaswdr/faker/internal/test" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need an internal package, the current structure is intentionally simple and only one single deep level
Since there is no interaction, I'm merging this to a separate branch and doing the changes myself. |
Description
100% test coverage for
person.go
.Are you trying to fix an existing issue?
Includes partial fixes for #28.
Go Version
Go Tests