refactor: allow random to be mocked

[xlanor]

Description

This uses an interface of GeneratorInterface type that implements Intn and Int (previously used directly from the rand package.

By doing this, we can now reliably mock rand by inserting expected values to test certain logic flows which may not have been fully covered.

This includes changes that are in the existing ethereum PR (#91) as I wrote the sample test code based on a method that was renamed in that PR.

If this style is approved I will push somemore commits to explicitly test the logic flow for RandomBitcoin

Are you trying to fix an existing issue?

#92

Go Version

abc@c63bccef3747:~/workspace/oss/faker$ go version
go version go1.17.3 linux/amd64

Go Tests

abc@c63bccef3747:~/workspace/oss/faker$ go test ./...
ok      github.com/jaswdr/faker 3.833s
abc@c63bccef3747:~/workspace/oss/faker$ 

[codecov]

Codecov Report

Merging #93 (4550059) into master (351f236) will decrease coverage by 0.46%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #93      +/-   ##
==========================================
- Coverage   96.31%   95.85%   -0.47%     
==========================================
  Files          36       36              
  Lines        1085     1085              
==========================================
- Hits         1045     1040       -5     
- Misses         34       36       +2     
- Partials        6        9       +3     

Impacted Files	Coverage Δ
faker.go	90.11% <ø> (ø)
person.go	92.59% <0.00%> (-7.41%)	⬇️
phone.go	92.85% <0.00%> (-7.15%)	⬇️
internet.go	91.15% <0.00%> (-1.77%)	⬇️
crypto.go	100.00% <0.00%> (+5.76%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 351f236...4550059. Read the comment docs.

[jaswdr]

I merged your previous PR @xlanor, can you rebase this one to resolve the conflicts?

[xlanor]

Will work on it by tonight, at work right now.

Does the implementation look good so far?

[xlanor]

resolved conflicts.

[xlanor]

@jaswdr sending a ping here :)

[jaswdr]

@xlanor sorry for the delay, there is a drop in coverage, can you check the "Files changes" tab to see the lines not covered?

[xlanor]

@xlanor sorry for the delay, there is a drop in coverage, can you check the "Files changes" tab to see the lines not covered?

Will do this weekend, been a busy month for me. Cheers

[xlanor]

@jaswdr coverage increased, rerun ci with merge.

Have a look at the test file to see how this can be used to essentially avoid the issue of decreasing coverage by mocking random (branch) logic.

There may be a slight drop ocassionally - need to refactor the code further in branching logics to allow for all branching to be fully tested, but that's a little hard due to the dependency on generating alnums and Intn both relying on the seed generator.

I suspect this is actually the case for dropping coverage across the board - it now reports a drop in coverage in
internet.go | 91.15% <0.00%> (-1.77%) even though the PR does not affect it.

I'll have a sleep on it and think about how to get it done.

With regards to the test failing here, this is actually in an unrelated file to mine and appears to be flaking.

func TestCurrencyNumber(t *testing.T) {
	c := New().Currency()
	NotExpect(t, 0, c.Number())
}

This tests expects a non-0 number

We can see that it calls Number() which returns this

// Number returns a random currency number
func (c Currency) Number() int {
	return c.Faker.RandomIntElement(currenciesNumbers)
}

This will flake because currencies Numbers contains a 0 which will randomly appear
currenciesNumbers = []int{971, 8, 12, 840, 978, 973, 951, 0, 951, 32, 51,...

Im not exactly sure whats the purpose of that test, could you look at it please?

[jaswdr]

Looking, those tests are clearly wrong, I'll fix them in another PR.