Merge pull request #496 from ahrtr/surgery_required_flags_20230516

cmd: mark some flags of surgery commands as required
etcd-io · May 16, 2023 · 9451390 · 9451390
2 parents bd6b79c + 065a3e1
commit 9451390
Show file tree

Hide file tree

Showing 2 changed files with 91 additions and 0 deletions.
diff --git a/cmd/bbolt/command_surgery.go b/cmd/bbolt/command_surgery.go
@@ -39,6 +39,7 @@ type surgeryBaseOptions struct {
 
 func (o *surgeryBaseOptions) AddFlags(fs *pflag.FlagSet) {
 	fs.StringVar(&o.outputDBFilePath, "output", o.outputDBFilePath, "path to the filePath db file")
+	_ = cobra.MarkFlagRequired(fs, "output")
 }
 
 func (o *surgeryBaseOptions) Validate() error {
@@ -101,6 +102,8 @@ func (o *surgeryCopyPageOptions) AddFlags(fs *pflag.FlagSet) {
 	o.surgeryBaseOptions.AddFlags(fs)
 	fs.Uint64VarP(&o.sourcePageId, "from-page", "", o.sourcePageId, "source page Id")
 	fs.Uint64VarP(&o.destinationPageId, "to-page", "", o.destinationPageId, "destination page Id")
+	_ = cobra.MarkFlagRequired(fs, "from-page")
+	_ = cobra.MarkFlagRequired(fs, "to-page")
 }
 
 func (o *surgeryCopyPageOptions) Validate() error {
@@ -172,6 +175,7 @@ type surgeryClearPageOptions struct {
 func (o *surgeryClearPageOptions) AddFlags(fs *pflag.FlagSet) {
 	o.surgeryBaseOptions.AddFlags(fs)
 	fs.Uint64VarP(&o.pageId, "pageId", "", o.pageId, "page Id")
+	_ = cobra.MarkFlagRequired(fs, "pageId")
 }
 
 func (o *surgeryClearPageOptions) Validate() error {
@@ -244,6 +248,9 @@ func (o *surgeryClearPageElementsOptions) AddFlags(fs *pflag.FlagSet) {
 	fs.Uint64VarP(&o.pageId, "pageId", "", o.pageId, "page id")
 	fs.IntVarP(&o.startElementIdx, "from-index", "", o.startElementIdx, "start element index (included) to clear, starting from 0")
 	fs.IntVarP(&o.endElementIdx, "to-index", "", o.endElementIdx, "end element index (excluded) to clear, starting from 0, -1 means to the end of page")
+	_ = cobra.MarkFlagRequired(fs, "pageId")
+	_ = cobra.MarkFlagRequired(fs, "from-index")
+	_ = cobra.MarkFlagRequired(fs, "to-index")
 }
 
 func (o *surgeryClearPageElementsOptions) Validate() error {

diff --git a/cmd/bbolt/command_surgery_test.go b/cmd/bbolt/command_surgery_test.go
@@ -674,3 +674,87 @@ func readPage(t *testing.T, path string, pageId int, pageSize int) []byte {
 func pageDataWithoutPageId(buf []byte) []byte {
 	return buf[8:]
 }
+
+func TestSurgeryRequiredFlags(t *testing.T) {
+	errMsgFmt := `required flag(s) "%s" not set`
+	testCases := []struct {
+		name           string
+		args           []string
+		expectedErrMsg string
+	}{
+		// --output is required for all surgery commands
+		{
+			name:           "no output flag for revert-meta-page",
+			args:           []string{"surgery", "revert-meta-page", "db"},
+			expectedErrMsg: fmt.Sprintf(errMsgFmt, "output"),
+		},
+		{
+			name:           "no output flag for copy-page",
+			args:           []string{"surgery", "copy-page", "db", "--from-page", "3", "--to-page", "2"},
+			expectedErrMsg: fmt.Sprintf(errMsgFmt, "output"),
+		},
+		{
+			name:           "no output flag for clear-page",
+			args:           []string{"surgery", "clear-page", "db", "--pageId", "3"},
+			expectedErrMsg: fmt.Sprintf(errMsgFmt, "output"),
+		},
+		{
+			name:           "no output flag for clear-page-element",
+			args:           []string{"surgery", "clear-page-elements", "db", "--pageId", "4", "--from-index", "3", "--to-index", "5"},
+			expectedErrMsg: fmt.Sprintf(errMsgFmt, "output"),
+		},
+		{
+			name:           "no output flag for freelist abandon",
+			args:           []string{"surgery", "freelist", "abandon", "db"},
+			expectedErrMsg: fmt.Sprintf(errMsgFmt, "output"),
+		},
+		{
+			name:           "no output flag for freelist rebuild",
+			args:           []string{"surgery", "freelist", "rebuild", "db"},
+			expectedErrMsg: fmt.Sprintf(errMsgFmt, "output"),
+		},
+		// --from-page and --to-page are required for 'surgery copy-page' command
+		{
+			name:           "no from-page flag for copy-page",
+			args:           []string{"surgery", "copy-page", "db", "--output", "db", "--to-page", "2"},
+			expectedErrMsg: fmt.Sprintf(errMsgFmt, "from-page"),
+		},
+		{
+			name:           "no to-page flag for copy-page",
+			args:           []string{"surgery", "copy-page", "db", "--output", "db", "--from-page", "2"},
+			expectedErrMsg: fmt.Sprintf(errMsgFmt, "to-page"),
+		},
+		// --pageId is required for 'surgery clear-page' command
+		{
+			name:           "no pageId flag for clear-page",
+			args:           []string{"surgery", "clear-page", "db", "--output", "db"},
+			expectedErrMsg: fmt.Sprintf(errMsgFmt, "pageId"),
+		},
+		// --pageId, --from-index and --to-index are required for 'surgery clear-page-element' command
+		{
+			name:           "no pageId flag for clear-page-element",
+			args:           []string{"surgery", "clear-page-elements", "db", "--output", "newdb", "--from-index", "3", "--to-index", "5"},
+			expectedErrMsg: fmt.Sprintf(errMsgFmt, "pageId"),
+		},
+		{
+			name:           "no from-index flag for clear-page-element",
+			args:           []string{"surgery", "clear-page-elements", "db", "--output", "newdb", "--pageId", "2", "--to-index", "5"},
+			expectedErrMsg: fmt.Sprintf(errMsgFmt, "from-index"),
+		},
+		{
+			name:           "no to-index flag for clear-page-element",
+			args:           []string{"surgery", "clear-page-elements", "db", "--output", "newdb", "--pageId", "2", "--from-index", "3"},
+			expectedErrMsg: fmt.Sprintf(errMsgFmt, "to-index"),
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			rootCmd := main.NewRootCommand()
+			rootCmd.SetArgs(tc.args)
+			err := rootCmd.Execute()
+			require.ErrorContains(t, err, tc.expectedErrMsg)
+		})
+	}
+}