Add Fuzz test for RSA

[HectorMalot]

As per #480 , my proposal to include fuzzing for RSA. Note that the test will run as a normal test if fuzzing is not enabled, so I didn't duplicate the code, but rather updated the test to allow fuzzing as well as normal testing.

Details of this change:

• Changed test from *Testing.T to *Testing.F
• Increased the testing prime numbers to larger numbers so that larger rune inputs also work. (i.e. up to MaxInt32)
• Included a unicode test case (that would fail on the previous test)

[tjgurwara99]

I personally am not in favour of removing old test and just having the fuzzing tests. I think both should be present.

[HectorMalot]

Hi @tjgurwara99,

That is how I started, but I realized that it would mean a lot of duplication, or a helper that contains all this code, called by nearly identical Test… and Fuzz… functions.

This is in the go documentation:

When fuzzing is disabled, the fuzz target is called with the seed inputs registered with F.Add and seed inputs from testdata/fuzz/. In this mode, the fuzz test acts much like a regular test, with subtests started with F.Fuzz instead of T.Run.

That gives me the impression that in this case, the proposed setup in the PR makes most sense. It functions as a normal test during a normal run, and can be used to fuzz when called with the fuzzing options.

Source: https://pkg.go.dev/testing#hdr-Fuzzing

Happy to change, but hope this makes you reconsider.

Cheers
Dennis

[tjgurwara99]

That is how I started, but I realized that it would mean a lot of duplication, or a helper that contains all this code, called by nearly identical Test… and Fuzz… functions.

Duplicate code for doing two separate things (in this case Fuzzing and Testing) are valid reasons for duplicating code.

@raklaptudirm @siriak what are your opinions?

I'm personally against having test and fuzzing mixed because of the inherent flaky nature of fuzzing.

[siriak]

Both ways are acceptable IMO

[HectorMalot]

***@***.***,That is how I started, but I realized that it would mean a lot of duplication, or a helper that contains all this code, called by nearly identical Test… and Fuzz… functions. This is in the go documentation: > When fuzzing is disabled, the fuzz target is called with the seed inputs registered with F.Add and seed inputs from testdata/fuzz/<Name>. In this mode, the fuzz test acts much like a regular test, with subtests started with F.Fuzz instead of T.Run.That convinced me that in this case, the proposed setup in the PR makes most sense. It functions as the normal test, and can be used to fuzz as well. Source: https://pkg.go.dev/testing#hdr-FuzzingHappy to change, but hope this makes you reconsider. CheersDennisOn 2 Oct 2022, at 19:00, Taj ***@***.***> wrote: @tjgurwara99 requested changes on this pull request. I personally am not in favour of removing old test and just having the fuzzing tests. I think both should be present. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>

[github-actions]

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

[github-actions]

This PR was closed because it has been stalled for 7 days with no activity.