tests/framework: Add GetTestFile #2136
Conversation
- Added a random uin32 identifier for a test, which will be used to identify a certain test run. Note these are random, but not unique. - GetTestFile is fed with a filename and returns a path to a file inside test folder. If a folder for current test does not exist, it is created in the process. Signed-off-by: Eduard Serra <eduser25@gmail.com>
| @@ -215,8 +242,14 @@ func (td *OsmTestData) AreRegistryCredsPresent() bool { | |||
| // Called by Gingkgo BeforeEach | |||
| func (td *OsmTestData) InitTestData(t GinkgoTInterface) error { | |||
| td.T = t | |||
| r, err := rand.Int(rand.Reader, big.NewInt(math.MaxUint32)) | |||
There was a problem hiding this comment.
How come we're not using time to seed the random number generator here? https://stackoverflow.com/a/12321192
There was a problem hiding this comment.
rand.crypto is supposed to be the safest, the reader from crypto is fed entropy from various sources I think.
In fact I was forced to change it to rand.crypto as using math.rand actually triggers the linter....
There was a problem hiding this comment.
For non crypto rand generations, using math.Rand is perfectly fine, you just need to silence the linter false alarms with // #nosec annotation
There was a problem hiding this comment.
@eduser25 - what is the linter error that you're getting?
There was a problem hiding this comment.
@shashankram, I see. the linter doesn't actually tell you it is fine for use for non-crypto.
@michelleN :
In some test file...
import (
"fmt"
"math/rand"
...
a := rand.Float32()
fmt.Printf("%i", a)
...
eserra@edu-linux ~/src/osm/tests (metrics) $ golangci-lint run --out-format=github-actions --timeout 5m
::error file=framework/common_profile.go,line=120,col=7::G404: Use of weak random number generator (math/rand instead of crypto/rand) (gosec)
There was a problem hiding this comment.
@shashankram, I see. the linter doesn't actually tell you it is fine for use for non-crypto.
That's because the linter can't know whether you are using the api for crypto purpose. Doesn't mean you need to use crypto api for non crypto operations, you just need to suppress the false alarm, no other alternative with gosec linter enabled.
Although the PR description has a one liner |
Basically tests saving some results, logs or test data on file/disk. Honestly thought it'd be more obvious, I'll add more purpose to description. |
We might be interested in near future to save test results, data or logs on disk.
This commit introduces a helper for tests to prefix filenames and create test folder
upon use.
Added a random uin64 identifier for a test, which will be used
to identify a certain test run. Note these are random, but not unique.
GetTestFile is fed with a filename and returns a path to a file
inside test folder. If a folder for current test does not exist, it is
created in the process.
Tests [X]
Does this change contain code from or inspired by another project? If so, did you notify the maintainers and provide attribution?
No