mirror of
https://github.com/d0zingcat/gocryptotrader.git
synced 2026-05-19 07:26:49 +00:00
config: Fix TestPromptForConfigEncryption race (#1929)
* Here's how I resolved a race condition in the encryption prompt tests:
I've refactored `promptForConfigEncryption` to accept an `io.Reader`. This allows tests to use `strings.NewReader` instead of relying on the global `os.Stdin`. This change isolates the input source for `TestPromptForConfigEncryption`, preventing concurrent access conflicts with `TestPromptForConfigKey` which uses `withInteractiveResponse` to manipulate `os.Stdin`.
The original issue manifested as a data race detected by `go test -race` between these two test functions due to their parallel execution (`t.Parallel()`) and their interaction with the shared `os.Stdin` resource.
Here are the changes I made:
- `config/config_encryption.go`:
- Modified `promptForConfigEncryption` to `promptForConfigEncryption(r io.Reader) (bool, error)`.
- Introduced `PromptForConfigEncryption()` as a public wrapper that calls the refactored function with `os.Stdin` for application use.
- `config/config.go`:
- Updated the call site for prompting config encryption to use the new `PromptForConfigEncryption()` wrapper.
- `config/config_encryption_test.go`:
- Updated `TestPromptForConfigEncryption` to call the (now unexported) `promptForConfigEncryption` with `strings.NewReader` for various input scenarios.
- `t.Parallel()` was maintained for `TestPromptForConfigEncryption`.
To verify, I confirmed that running `go test -race ./config/...` shows the previously reported race condition is no longer present. All tests in the `config` package now pass with the race detector enabled.
* Refactor(config): Remove redundant loop variable capture in test
I've removed the explicit `tc := tc` line in `TestPromptForConfigEncryption`
as it is no longer necessary for Go versions 1.22 and later.
The project's Go version (1.24.3 as per CI) handles loop variable
scoping correctly for parallel subtests, making this capture redundant.
This change is a follow-up to the fix for the race condition in issue #1928,
addressing your feedback on code style. No functional changes are introduced
by this commit.
* Style(config): Remove explicit empty string in test case
Refactors the "input_empty_eof" test case in
`TestPromptForConfigEncryption` to remove the explicit
assignment of `input: ""`. This relies on Go's default
behavior for struct field initialization (a string field
defaults to an empty string), making the code more concise.
This change is a follow-up to previous refactorings for issue #1928,
addressing your feedback on code style. No functional changes
are introduced by this commit.
* Update config/config_encryption_test.go
Co-authored-by: Ryan O'Hara-Reid <oharareid.ryan@gmail.com>
* linter: Make indenting a happy bing
* config: Rid PromptForConfigEncryption
---------
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Co-authored-by: Ryan O'Hara-Reid <oharareid.ryan@gmail.com>
This commit is contained in:
@@ -1487,7 +1487,7 @@ func (c *Config) readConfig(d io.Reader) error {
|
||||
// If they agree, c.EncryptConfig is set to Enabled, the config is encrypted and saved
|
||||
// Otherwise, c.EncryptConfig is set to Disabled and the file is resaved
|
||||
func (c *Config) saveWithEncryptPrompt(path string) error {
|
||||
if confirm, err := promptForConfigEncryption(); err != nil {
|
||||
if confirm, err := promptForConfigEncryption(os.Stdin); err != nil {
|
||||
return nil //nolint:nilerr // Ignore encryption prompt failures; The user will be prompted again
|
||||
} else if confirm {
|
||||
c.EncryptConfig = fileEncryptionEnabled
|
||||
|
||||
@@ -43,11 +43,11 @@ var (
|
||||
|
||||
// promptForConfigEncryption asks for encryption confirmation
|
||||
// returns true if encryption was desired, false otherwise
|
||||
func promptForConfigEncryption() (bool, error) {
|
||||
func promptForConfigEncryption(r io.Reader) (bool, error) {
|
||||
fmt.Println("Would you like to encrypt your config file (y/n)?")
|
||||
|
||||
input := ""
|
||||
if _, err := fmt.Scanln(&input); err != nil {
|
||||
if _, err := fmt.Fscanln(r, &input); err != nil {
|
||||
return false, err
|
||||
}
|
||||
|
||||
|
||||
@@ -19,9 +19,49 @@ import (
|
||||
func TestPromptForConfigEncryption(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
confirm, err := promptForConfigEncryption()
|
||||
require.ErrorIs(t, err, io.EOF)
|
||||
require.False(t, confirm)
|
||||
testCases := []struct {
|
||||
name string
|
||||
input string
|
||||
expectedBool bool
|
||||
expectedError error
|
||||
}{
|
||||
{
|
||||
name: "input_y",
|
||||
input: "y\n",
|
||||
expectedBool: true,
|
||||
},
|
||||
{
|
||||
name: "input_n",
|
||||
input: "n\n",
|
||||
},
|
||||
{
|
||||
name: "input_yes",
|
||||
input: "yes\n",
|
||||
expectedBool: true,
|
||||
},
|
||||
{
|
||||
name: "input_no",
|
||||
input: "no\n",
|
||||
},
|
||||
{
|
||||
name: "input_invalid",
|
||||
input: "invalid\n",
|
||||
},
|
||||
{
|
||||
name: "input_empty_eof",
|
||||
expectedError: io.EOF,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
reader := strings.NewReader(tc.input)
|
||||
confirm, err := promptForConfigEncryption(reader)
|
||||
require.ErrorIs(t, err, tc.expectedError)
|
||||
require.Equal(t, tc.expectedBool, confirm)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestPromptForConfigKey(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user