testutil: replace math/rand deprecation

[KaloyanTanev]

• Update global rand.Seed(x) with local rand.New(rand.NewSource(x));
• Update test random functions that used global seed with local seed;
• Update math/rand.Read(x) with crypto/rand.Read(x) (not included in the issue, but deprecated as well).

category: refactor
ticket: #2831

[pinebit]

@KaloyanTanev thank you for the contribution. You can run pre-commit locally before making a commit to determine possible issues or potential test failures.
Please check and address the issues reported by linter:

  Error: Error return value of `rand.Read` is not checked (errcheck)
  Error: test helper function should start from t.Helper() (thelper)
  Error: test helper function should start from t.Helper() (thelper)
  Error: test helper function should start from t.Helper() (thelper)

p.s. some tests are known to be flakey, like *qbft* and TestSimnetDuties/sync_committee_with_teku.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cfe844b) 53.44% compared to head (2acc741) 53.40%.
Report is 2 commits behind head on main.

❗ Current head 2acc741 differs from pull request most recent head 1332a6b. Consider uploading reports for the commit 1332a6b to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2850      +/-   ##
==========================================
- Coverage   53.44%   53.40%   -0.05%     
==========================================
  Files         200      200              
  Lines       28011    28034      +23     
==========================================
+ Hits        14971    14972       +1     
- Misses      11180    11208      +28     
+ Partials     1860     1854       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[KaloyanTanev]

Hi @pinebit , wasn't aware of the pre-commit, thanks. I've ran only go test ./... -failfast -race locally. Is there anything else I should check before pushing?

Fixed the linting.

[xenowits]

@KaloyanTanev you should install pre-commit pre-commit install and then run pre-commit run --all-files

This should report all possible linting errors that you're having rn

[gsora]

LFTM, good to merge once linter issues are fixed.

[pinebit]

@KaloyanTanev
The linter is still reporting a few issues:

  Error: test helper function should start from t.Helper() (thelper)
  Error: test helper function should start from t.Helper() (thelper)
  Error: test helper function should start from t.Helper() (thelper)

[sonarcloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
2.0% Duplication on New Code

See analysis details on SonarCloud

[KaloyanTanev]

Hmmm, a bit weird as locally the pre-commit run --all-files passed.

➜  charon git:(kalo/update-deprecated-seed) ✗ pre-commit run --all-files
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
mixed line ending........................................................Passed
don't commit to branch...................................................Passed
check for go version.....................................................Passed
check if file has licence text appended as header........................Passed
fiximports...............................................................Passed
go-mod-tidy..............................................................Passed
go fmt...................................................................Passed
golangci-lint............................................................Passed
run-go-tests.............................................................Passed
run-buf..................................................................Passed
run-regexp...............................................................Passed
run-testutil.............................................................Passed

However, running golangci-lint on its own gave me the expected linting errors:

➜  charon git:(kalo/update-deprecated-seed) ✗ golangci-lint run   
testutil/random.go:1205:6: test helper function should start from t.Helper() (thelper)
func RandomCoreAttestationData(t *testing.T) core.AttestationData {
     ^
testutil/random.go:1221:6: test helper function should start from t.Helper() (thelper)
func RandomUnsignedDataSet(t *testing.T) core.UnsignedDataSet {
     ^
cluster/test_cluster.go:188:6: test helper function should start from t.Helper() (thelper)
func RandomRegistration(t *testing.T, network string) BuilderRegistration {
     ^

I think it should be good after latest commit.

[pinebit]

Hmmm, a bit weird as locally the pre-commit run --all-files passed.

Thank you @KaloyanTanev for the effort. Running a new build now, and if it succeeds we will get it merged.
Good job on the ticket!

[KaloyanTanev]

@pinebit I see one of the unit tests failed - TestRawRouter/get_validators_with_post/empty_parameters. Is it one of the known flaky tests or shall I have a look at it?

[pinebit]

@pinebit I see one of the unit tests failed - TestRawRouter/get_validators_with_post/empty_parameters. Is it one of the known flaky tests or shall I have a look at it?

This seems to be one bug I likely introduced today :(
JSON serialization is not stable, therefore these checks need to be improved (we cannot rely on 0 and 1 indexes)

			require.EqualValues(t, eth2p0.ValidatorIndex(12), resp.Data[0].Index)
			require.EqualValues(t, eth2p0.ValidatorIndex(35), resp.Data[1].Index)

in router_internal_test.go

[pinebit]

@KaloyanTanev I created separate PR for this issue #2857 because our main is also broken. Your build should succeed eventually.