ronantakizawa/codereview-bench huggingface dataset:
Repository: spf13/cobra
File: cobra_test.go (Go)

Code before:
			}
		})
	}
}

func TestStringInSlice(t *testing.T) {
	tests := []struct {
		name     string
		a        string
		list     []string
		expected bool
	}{
		{
			name:     "String in slice (case-sensitive)",
			a:        "apple",
			list:     []string{"orange", "banana", "apple", "grape"},
			expected: true,
		},
		{
			name:     "String not in slice (case-sensitive)",
			a:        "pear",
			list:     []string{"orange", "banana", "apple", "grape"},
			expected: false,
		},
		{
			name:     "String in slice (case-insensitive)",
			a:        "APPLE",
			list:     []string{"orange", "banana", "apple", "grape"},
			expected: false,
		},
		{
			name:     "String not in slice (case-insensitive)",
			a:        "PEAR",
			list:     []string{"orange", "banana", "apple", "grape"},
			expected: false,
		},
		{
			name:     "Empty slice",
			a:        "apple",
			list:     []string{},
			expected: false,
		},
		{
			name:     "Empty string",
			a:        "",
			list:     []string{"orange", "banana", "apple", "grape"},
			expected: false,
		},
		{
			name:     "Empty string in empty slice",
			a:        "",

Review comment (security):
I don't think we need case-insensitive tests since the function being testes is case-sensitive.
So, for this test, since `apple` does not match in the test above, do we really need to test `APPLE`?

Code after:
			}
		})
	}
}

func TestStringInSlice(t *testing.T) {
	tests := []struct {
		name     string
		a        string
		list     []string
		expected bool
	}{
		{
			name:     "String in slice (case-sensitive)",
			a:        "apple",
			list:     []string{"orange", "banana", "apple", "grape"},
			expected: true,
		},
		{
			name:     "String not in slice (case-sensitive)",
			a:        "pear",
			list:     []string{"orange", "banana", "apple", "grape"},
			expected: false,
		},
		{
			name:     "String in slice (case-insensitive)",
			a:        "APPLE",
			list:     []string{"orange", "banana", "apple", "grape"},
			expected: false,
		},
		{
			name:     "Empty slice",
			a:        "apple",
			list:     []string{},
			expected: false,
		},
		{
			name:     "Empty string",
			a:        "",
			list:     []string{"orange", "banana", "apple", "grape"},
			expected: false,
		},
		{
			name:     "Empty strings match",
			a:        "",
			list:     []string{"orange", ""},
			expected: true,
		},
		{
			name:     "Empty string in empty slice",
			a:        "",
Repository: spf13/cobra
File: command_go120.go (Go)

Code before:
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//      http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

//go:build !go1.21
// +build !go1.21

package cobra

import (
	"os"
	"strings"
)

// based on golang.org/x/mod/internal/lazyregexp: https://cs.opensource.google/go/x/mod/+/refs/tags/v0.19.0:internal/lazyregexp/lazyre.go;l=66
var inTest = len(os.Args) > 0 && strings.HasSuffix(strings.TrimSuffix(os.Args[0], ".exe"), ".test")

func isTesting() bool {
	return inTest
}


Review comment (bug):
So this will break programs using Cobra with go < 1.21 which are named `<something>.test`.

For such a very rare case, what is the workaround?  

Code after:
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//      http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

//go:build !go1.21
// +build !go1.21

package cobra

import (
	"os"
	"strings"
)

// based on golang.org/x/mod/internal/lazyregexp: https://cs.opensource.google/go/x/mod/+/refs/tags/v0.19.0:internal/lazyregexp/lazyre.go;l=66
// For a non-go-test program which still has a name ending with ".test[.exe]", it will need to either:
// 1- Use go >= 1.21, or
// 2- call "rootCmd.SetArgs(os.Args[1:])" before calling "rootCmd.Execute()"
var inTest = len(os.Args) > 0 && strings.HasSuffix(strings.TrimSuffix(os.Args[0], ".exe"), ".test")

func isTesting() bool {
	return inTest
}
Repository: spf13/cobra
File: command_test.go (Go)

Code before:
			checkStringContains(t, output, "unknown flag: --unknown")
		})
	}
}

func TestHelpFuncExecuted(t *testing.T) {
	helpExecuted := false

	child := &Command{Use: "child", Long: "Long description", Run: emptyRun}
	child.SetHelpFunc(func(cmd *Command, args []string) {
		helpExecuted = true

		if cmd.Context() == nil {
			t.Error("Context is nil")
		}
	})

	rootCmd := &Command{Use: "root", Run: emptyRun}
	rootCmd.AddCommand(child)

	output, err := executeCommand(rootCmd, "help", "child")
	if err != nil {
		t.Errorf("Unexpected error: %v", err)
	}

	checkStringContains(t, output, rootCmd.Long)
	if !helpExecuted {
		t.Error("Help function not executed")
	}
}


Review comment (suggestion):
I don't get this check here...
`rootCmd.Long` is empty; I assume you meant to use `child.Long`?
But then the `output` is also empty because the `HelpFunc` defined above does not actually print any help message.  I assume it should, so you can verify it?  Then you won't need to do the `helpExecuted` check.

Code after:
			checkStringContains(t, output, "unknown flag: --unknown")
		})
	}
}

func TestHelpFuncExecuted(t *testing.T) {
	helpText := "Long description"

	// Create a context that will be unique, not just the background context
	//nolint:golint,staticcheck // We can safely use a basic type as key in tests.
	executionCtx := context.WithValue(context.Background(), "testKey", "123")

	child := &Command{Use: "child", Run: emptyRun}
	child.SetHelpFunc(func(cmd *Command, args []string) {
		_, err := cmd.OutOrStdout().Write([]byte(helpText))
		if err != nil {
			t.Error(err)
		}

		// Test for https://github.com/spf13/cobra/issues/2240
		if cmd.Context() != executionCtx {
			t.Error("Context doesn't equal the execution context")
		}
	})

	rootCmd := &Command{Use: "root", Run: emptyRun}
	rootCmd.AddCommand(child)

	output, err := executeCommandWithContext(executionCtx, rootCmd, "help", "child")
	if err != nil {
		t.Errorf("Unexpected error: %v", err)
	}

	checkStringContains(t, output, helpText)
}
Repository: spf13/cobra
File: command.go (Go)

Code before:
				var completions []Completion
				cmd, _, e := c.Root().Find(args)
				if e != nil {
					return nil, ShellCompDirectiveNoFileComp
				}
				if cmd == nil {
					// Root help command.
					cmd = c.Root()
				}
				for _, subCmd := range cmd.Commands() {
					if subCmd.IsAvailableCommand() || subCmd == cmd.helpCommand {
						if strings.HasPrefix(subCmd.Name(), toComplete) {
							completions = append(completions, CompletionWithDesc(subCmd.Name(), subCmd.Short))
						}
					}
				}
				return completions, ShellCompDirectiveNoFileComp
			},
			Run: func(c *Command, args []string) {
				cmd, _, e := c.Root().Find(args)
				if cmd == nil || e != nil {
					c.Printf("Unknown help topic %#q\n", args)
					CheckErr(c.Root().Usage())
				} else {
					// FLow the context down to be used in help text
					cmd.ctx = c.ctx

					cmd.InitDefaultHelpFlag()    // make possible 'help' flag to be shown
					cmd.InitDefaultVersionFlag() // make possible 'version' flag to be shown
					CheckErr(cmd.Help())
				}
			},
			GroupID: c.helpCommandGroupID,
		}
	}
	c.RemoveCommand(c.helpCommand)
	c.AddCommand(c.helpCommand)
}

// ResetCommands delete parent, subcommand and help command from c.
func (c *Command) ResetCommands() {
	c.parent = nil
	c.commands = nil
	c.helpCommand = nil
	c.parentsPflags = nil
}

// Sorts commands by their names.
type commandSorterByName []*Command

func (c commandSorterByName) Len() int           { return len(c) }

Review comment (suggestion):
Just to be safe can you wrap this in a `if cmd.ctx == nil`?  This is how it is done elsewhere and I'm not sure if the `cmd` could somehow already have a context set, so let's be safe.

Code after:
				var completions []Completion
				cmd, _, e := c.Root().Find(args)
				if e != nil {
					return nil, ShellCompDirectiveNoFileComp
				}
				if cmd == nil {
					// Root help command.
					cmd = c.Root()
				}
				for _, subCmd := range cmd.Commands() {
					if subCmd.IsAvailableCommand() || subCmd == cmd.helpCommand {
						if strings.HasPrefix(subCmd.Name(), toComplete) {
							completions = append(completions, CompletionWithDesc(subCmd.Name(), subCmd.Short))
						}
					}
				}
				return completions, ShellCompDirectiveNoFileComp
			},
			Run: func(c *Command, args []string) {
				cmd, _, e := c.Root().Find(args)
				if cmd == nil || e != nil {
					c.Printf("Unknown help topic %#q\n", args)
					CheckErr(c.Root().Usage())
				} else {
					// FLow the context down to be used in help text
					if cmd.ctx == nil {
						cmd.ctx = c.ctx
					}

					cmd.InitDefaultHelpFlag()    // make possible 'help' flag to be shown
					cmd.InitDefaultVersionFlag() // make possible 'version' flag to be shown
					CheckErr(cmd.Help())
				}
			},
			GroupID: c.helpCommandGroupID,
		}
	}
	c.RemoveCommand(c.helpCommand)
	c.AddCommand(c.helpCommand)
}

// ResetCommands delete parent, subcommand and help command from c.
func (c *Command) ResetCommands() {
	c.parent = nil
	c.commands = nil
	c.helpCommand = nil
	c.parentsPflags = nil
}

// Sorts commands by their names.
type commandSorterByName []*Command
Repository: spf13/cobra
File: command_test.go (Go)

Code before:
	root.PersistentFlags().String("namespace", "", "a string flag")

	c := &Command{
		Use: "child",
		Run: emptyRun,
	}
	c.Flags().Bool("bar", false, "a boolean flag")

	root.AddCommand(c)

	for _, tc := range testCases {
		t.Run(strings.Join(tc, " "), func(t *testing.T) {
			output, err := executeCommand(root, tc...)
			if err == nil {
				t.Error("expected unknown flag error")
			}
			checkStringContains(t, output, "unknown flag: --unknown")
		})
	}
}

func TestHelpFuncExecuted(t *testing.T) {
	helpText := "Long description"

	// Create a context that will be unique, not just the background context
	executionCtx, cancel := context.WithCancel(context.Background())
	defer cancel()

	child := &Command{Use: "child", Run: emptyRun}
	child.SetHelpFunc(func(cmd *Command, args []string) {
		_, err := cmd.OutOrStdout().Write([]byte(helpText))
		if err != nil {
			t.Error(err)
		}

		// Test for https://github.com/spf13/cobra/issues/2240
		if cmd.Context() != executionCtx {
			t.Error("Context doesn't equal the execution context")
		}
	})

	rootCmd := &Command{Use: "root", Run: emptyRun}
	rootCmd.AddCommand(child)

	output, err := executeCommandWithContext(executionCtx, rootCmd, "help", "child")
	if err != nil {
		t.Errorf("Unexpected error: %v", err)
	}

	checkStringContains(t, output, helpText)
}

Review comment (bug):
The linter is failing because the context is not being canceled. You can use a context with a value instead `ctx := context.WithValue(context.Background(), "testKey", "123")`
You can copy this from the test `TestValidArgsFuncCmdContext`

Code after:
	root.PersistentFlags().String("namespace", "", "a string flag")

	c := &Command{
		Use: "child",
		Run: emptyRun,
	}
	c.Flags().Bool("bar", false, "a boolean flag")

	root.AddCommand(c)

	for _, tc := range testCases {
		t.Run(strings.Join(tc, " "), func(t *testing.T) {
			output, err := executeCommand(root, tc...)
			if err == nil {
				t.Error("expected unknown flag error")
			}
			checkStringContains(t, output, "unknown flag: --unknown")
		})
	}
}

func TestHelpFuncExecuted(t *testing.T) {
	helpText := "Long description"

	// Create a context that will be unique, not just the background context
	//nolint:golint,staticcheck // We can safely use a basic type as key in tests.
	executionCtx := context.WithValue(context.Background(), "testKey", "123")

	child := &Command{Use: "child", Run: emptyRun}
	child.SetHelpFunc(func(cmd *Command, args []string) {
		_, err := cmd.OutOrStdout().Write([]byte(helpText))
		if err != nil {
			t.Error(err)
		}

		// Test for https://github.com/spf13/cobra/issues/2240
		if cmd.Context() != executionCtx {
			t.Error("Context doesn't equal the execution context")
		}
	})

	rootCmd := &Command{Use: "root", Run: emptyRun}
	rootCmd.AddCommand(child)

	output, err := executeCommandWithContext(executionCtx, rootCmd, "help", "child")
	if err != nil {
		t.Errorf("Unexpected error: %v", err)
	}

	checkStringContains(t, output, helpText)
}
Repository: spotify/backstage
File: plugins/catalog-backend-module-backstage-openapi/config.d.ts (TypeScript)

Code before:
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

export interface Config {
  catalog?: {
    providers?: {
      /**
       * BackstageOpenApiEntityProvider configuration
       */
      backstageOpenapi?: {
        /**
         * A list of plugins, whose OpenAPI specs you want to collate in `InternalOpenApiDocumentationProvider`.
         */
        plugins: string[];
        /**
         * Properties to override on the final entity object.
         */
        entityOverrides?: object;
        /**
         * The format of the definition.
         * @defaultValue json
         */
        definitionFormat: 'json' | 'yaml';
      };
    };
  };
}


Review comment (bug):
Does this need to be `definitionFormat?: 'json' | 'yaml'` otherwise it might fail schema validation?

Code after:
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

export interface Config {
  catalog?: {
    providers?: {
      /**
       * BackstageOpenApiEntityProvider configuration
       */
      backstageOpenapi?: {
        /**
         * A list of plugins, whose OpenAPI specs you want to collate in `InternalOpenApiDocumentationProvider`.
         */
        plugins: string[];
        /**
         * Properties to override on the final entity object.
         */
        entityOverrides?: object;
        /**
         * The format of the definition.
         * @defaultValue json
         */
        definitionFormat?: 'json' | 'yaml';
      };
    };
  };
}
Repository: spotify/backstage
File: packages/integration/src/azure/DefaultAzureDevOpsCredentialsProvider.test.ts (TypeScript)

Code before:
                  },
                ],
              },
            ]);

            const credentials = await provider.getCredentials({
              url: formatUrl({
                host: 'my.other.devops.server',
                organization: 'org1',
                project: 'project1',
              }),
            });

            expect(credentials).toBeUndefined();
          });
        });
      });
    });
    describe('Azure DevOps (visualstudio.com)', () => {
      it('Should return a token when a credential with the same organization is specified', async () => {
        const provider = buildProvider([
          {
            host: 'org1.visualstudio.com',
            credentials: [
              {
                organizations: ['org1'],
                personalAccessToken: 'pat',
              },
            ],
          },
        ]);

        const credentials = provider.getCredentials({
          url: 'https://org1.visualstudio.com/project1',
        });

        expect(credentials).toBeDefined();
      });
    });
  });
});


Review comment (security):
You could argue that this field is not needed since it is embedded in the URL in this case. But I'm not sure how hard that is to enforce in the configuration schema. The reason I added the `organizations` field is because `dev.azure.com` applies to all organizations but the `.visualstudio.com` URLs are organization specific.

It is a potential point for misconfiguration but, if I remember correctly, you can also leave it out and then this credential would apply to "all orgs" for `org1.visualstudio.com` (which technically is only `org1`).
```suggestion
```

Code after:
                  },
                ],
              },
            ]);

            const credentials = await provider.getCredentials({
              url: formatUrl({
                host: 'my.other.devops.server',
                organization: 'org1',
                project: 'project1',
              }),
            });

            expect(credentials).toBeUndefined();
          });
        });
      });
    });
    describe('Azure DevOps (visualstudio.com)', () => {
      it('Should return a token when a credential with the same organization is specified', async () => {
        const provider = buildProvider([
          {
            host: 'org1.visualstudio.com',
            credentials: [
              {
                personalAccessToken: 'pat',
              },
            ],
          },
        ]);

        const credentials = provider.getCredentials({
          url: 'https://org1.visualstudio.com/project1',
        });

        expect(credentials).toBeDefined();
      });
    });
  });
});
Repository: spotify/backstage
File: packages/integration/src/azure/core.ts (TypeScript)

Code before:
 * @param url - A URL pointing to a path
 * @public
 */
export function getAzureDownloadUrl(url: string): string {
  return AzureUrl.fromRepoUrl(url).toArchiveUrl();
}

/**
 * Given a URL, return the API URL to fetch commits on the branch.
 *
 * @param url - A URL pointing to a repository or a sub-path
 * @public
 */
export function getAzureCommitsUrl(url: string): string {
  return AzureUrl.fromRepoUrl(url).toCommitsUrl();
}

/**
 * Given a URL, return true if it contains `visualstudio` and false if it does not
 * URLs cna be in these two formats: `dev.azure.com/{org}` or the legacy `{org}.visualstudio.com`
 *
 * @param origin - A URL origin string pointing to an Azure DevOps instance
 * @public
 */
export function isVisualStudioDomain(origin: string): boolean {
  return origin.split('.')[1] === 'visualstudio';
}


Review comment (suggestion):
Is this always and origin and are visualstudio domains always .com, in that case it might be cleaner with `origin.endsWith('visualstudio.com')`. There's a potential issue here where submitting something like `https://foo.visualstudio.evil.hacker.site.com` could be troublesome. Thinking it would be good to make this a bit more strict in some way, just for safety.

Code after:
 * @param url - A URL pointing to a path
 * @public
 */
export function getAzureDownloadUrl(url: string): string {
  return AzureUrl.fromRepoUrl(url).toArchiveUrl();
}

/**
 * Given a URL, return the API URL to fetch commits on the branch.
 *
 * @param url - A URL pointing to a repository or a sub-path
 * @public
 */
export function getAzureCommitsUrl(url: string): string {
  return AzureUrl.fromRepoUrl(url).toCommitsUrl();
}

/**
 * Given a URL, return true if it contains `.visualstudio.com` and false if it does not
 * URLs can be in these two formats: `dev.azure.com/{org}` or the legacy `{org}.visualstudio.com`
 *
 * @param origin - A URL origin string pointing to an Azure DevOps instance
 * @public
 */
export function isVisualStudioDomain(origin: string): boolean {
  return origin.endsWith('.visualstudio.com');
}
Repository: spotify/backstage
File: plugins/techdocs-node/src/stages/publish/awsS3.test.ts (TypeScript)

Code before:
    },
    'some folder': {
      'also with spaces.js': 'found it too',
    },
  };

  beforeEach(async () => {
    process.env = { ...env };
    process.env.AWS_REGION = 'us-west-2';

    jest.resetAllMocks();
    getCredProviderMock.mockImplementation((_?: AwsCredentialProviderOptions) =>
      getMockCredentialProvider(),
    );

    // Create a fresh mockdirectory for each test to avoid windows file locking
    mockDir = createMockDirectory();
    // Calculate directory path with the new mockDir instance
    directory = getEntityRootDir(entity);

    // Set up the test files
    mockDir.setContent({
      [directory]: files,
    });

    s3Mock = mockClient(S3Client as any);

    s3Mock.on(HeadObjectCommand).callsFake((input: any) => {
      if (!fs.pathExistsSync(mockDir.resolve(input.Key))) {
        throw new Error('File does not exist');
      }
      return {};
    });

    s3Mock.on(GetObjectCommand).callsFake((input: any) => {
      if (fs.pathExistsSync(mockDir.resolve(input.Key))) {
        return {
          Body: Readable.from(fs.readFileSync(mockDir.resolve(input.Key))),
        };
      }

      throw new Error(`The file ${input.Key} does not exist!`);
    });

    s3Mock.on(HeadBucketCommand).callsFake((input: any) => {
      if (input.Bucket === 'errorBucket') {
        throw new Error('Bucket does not exist');
      }
      return {};
    });


Review comment (suggestion):
```suggestion
    s3Mock = mockClient(S3Client);
```

Code after:
    img: {
      'with spaces.png': 'found it',
      'unsafe.svg': '<svg></svg>',
    },
    'some folder': {
      'also with spaces.js': 'found it too',
    },
  };

  beforeEach(async () => {
    process.env = { ...env };
    process.env.AWS_REGION = 'us-west-2';

    jest.resetAllMocks();
    getCredProviderMock.mockImplementation((_?: AwsCredentialProviderOptions) =>
      getMockCredentialProvider(),
    );

    // Create a fresh mockdirectory for each test to avoid windows file locking
    mockDir = createMockDirectory();
    // Calculate directory path with the new mockDir instance
    directory = getEntityRootDir(entity);

    // Set up the test files
    mockDir.setContent({
      [directory]: files,
    });

    s3Mock = mockClient(S3Client);

    s3Mock.on(HeadObjectCommand).callsFake(input => {
      if (!fs.pathExistsSync(mockDir.resolve(input.Key))) {
        throw new Error('File does not exist');
      }
      return {};
    });

    s3Mock.on(GetObjectCommand).callsFake(input => {
      if (fs.pathExistsSync(mockDir.resolve(input.Key))) {
        return {
          Body: Readable.from(fs.readFileSync(mockDir.resolve(input.Key))),
        };
      }

      throw new Error(`The file ${input.Key} does not exist!`);
    });

    s3Mock.on(HeadBucketCommand).callsFake(input => {
      if (input.Bucket === 'errorBucket') {
        throw new Error('Bucket does not exist');
      }