Skip to content
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.

Sign up for GitHub

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

Jump to bottom

deprecate unneeded utils.Testing #4350

Merged
merged 1 commit into from
Sep 6, 2024
Merged

deprecate unneeded utils.Testing #4350

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions utils/value.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package utils

import (
"flag"
"math/rand"
"os"
"strings"
"testing"
)

// AssertType attempts to assert that the given interface argument is
Expand Down Expand Up @@ -46,9 +46,9 @@ type Rand interface {
}

// Testing returns true when you are running in test suite.
// Deprecated: this is in the standard library now.
func Testing() bool {
// TODO switch to official testing.Testing method when we are on go 1.21
return flag.Lookup("test.v") != nil
return testing.Testing()
}
Comment on lines 48 to 52
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt this is being used by external users. Seems like it is only used internally in utils/test_detector/main.go

Not sure how we go about deleting unused but public code. (we are pre v1 so maybe find to delete)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally do one release with it deprecated, next release removes? can be convinced to delete now if you think that's the move

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I doubt people are upgrading every single week to the new version. Unless we want to deprecate this now and remove in 1-2 months (or never remove it), I doubt it makes a difference if we wait a week to remove.

Safest is just to deprecate. But I would hope that we could just remove.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not too involved in rdk releases nowadays

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will leave as-is and make it a problem for future me


// randWrapper is a pass-through to the shared math/rand functions.
Expand All @@ -61,7 +61,7 @@ func (randWrapper) Float64() float64 {
// SafeTestingRand returns a wrapper around the shared math/rand source in prod,
// and a deterministic rand.Rand seeded with 0 in test.
func SafeTestingRand() Rand {
if Testing() {
if testing.Testing() {
return rand.New(rand.NewSource(0)) //nolint:gosec
}
return randWrapper{}
Expand Down
Loading