-
-
Notifications
You must be signed in to change notification settings - Fork 298
feat: add global non-interactive mode support #905
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.
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
base: main
Are you sure you want to change the base?
feat: add global non-interactive mode support #905
Conversation
Add --no-input flag and no_input config setting to disable all interactive prompts. When enabled, commands fail immediately with descriptive errors instead of prompting for missing required fields, making the tool suitable for programmatic usage, scripts, and CI/CD pipelines. The feature can be enabled via: - CLI flag: --no-input - Config file: no_input: true in ~/.jira/.config.yml - Environment variable: JIRA_NO_INPUT=true All commands that require user input now support non-interactive mode and exit with status code 1 when required fields are missing.
- Add TestShallOverwrite table-driven test with both true/false cases - Replace trivial TestErrNoInputRequired with TestConfigureInstallationTypeNoInput that verifies actual feature behavior (error on missing required field) - Add TestNoInputConfigurationMethods to verify all three config methods work: environment variable (JIRA_NO_INPUT), config file (viper.Set), and precedence Addresses code review recommendations for more meaningful test coverage.
- Add error checking for os.Setenv/os.Unsetenv calls in configuration tests - Remove unused originalBody parameter from hasNoChanges function - Improve test coverage for no-input configuration methods
internal/cmdutil/utils_test.go
Outdated
| os.Setenv("JIRA_NO_INPUT", "true") | ||
| }, | ||
| cleanup: func() { | ||
| os.Unsetenv("JIRA_NO_INPUT") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can directly use t.SetEnv
There is no need for cleaning up
It would also fix the linting issues you have right now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried to address this in da24dda.
internal/cmdutil/utils_test.go
Outdated
| viper.Reset() | ||
| viper.AutomaticEnv() | ||
| viper.SetEnvPrefix("jira") | ||
| os.Unsetenv("JIRA_NO_INPUT") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one could be removed
| os.Unsetenv("JIRA_NO_INPUT") |
I feel like you added it for avoiding possible issues with parallel testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried to address this in da24dda.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't have to remove the parallel testing. t.SetEnv is safe while os.SetEmv wasn't
internal/cmd/issue/edit/edit.go
Outdated
| } | ||
|
|
||
| // hasNoChanges checks if any editable fields were provided in the params. | ||
| func hasNoChanges(params *editParams) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be this?
| func hasNoChanges(params *editParams) bool { | |
| func hasNoChanges(params *editParams) bool { |
And so, it could become a method
| func hasNoChanges(params *editParams) bool { | |
| func (params *editParams) hasNoChanges() bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried to address this in da24dda.
- Use t.Setenv() for cleaner test setup - Convert hasNoChanges to method receiver - Remove t.Parallel() from environment variable tests - Remove unnecessary t.Cleanup calls
|
@ccoVeille Thank you for the careful review. Go is not my preferred language and it showed here! Appreciate the help. Pushed da24dda with your suggestions. |
- Remove t.Parallel() from tests using viper.Reset() (not thread-safe) - Tests now run sequentially to avoid data races on viper singleton
|
The issues in the DeepSource: Go checks don't appear to have been introduced by my changes. |
Cherry-picked from idleyoungman/feat/global-no-input-mode Original author: Dan Young
Add
--no-inputflag andno_inputconfig setting to disable all interactiveprompts. When enabled, commands fail immediately with descriptive errors
instead of prompting for missing required fields, making the tool suitable
for programmatic usage, scripts, and CI/CD pipelines.
The feature can be enabled via:
--no-inputno_input: truein~/.jira/.config.ymlJIRA_NO_INPUT=trueAll commands that require user input now support non-interactive mode and
exit with status code 1 when required fields are missing.