-
Couldn't load subscription status.
- Fork 50
test: Reuse Bitbucket repos #1912
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?
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test all |
|
/gemini review |
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.
Code Review
This pull request is a great improvement, refactoring the Bitbucket e2e test provider to reuse repositories and replacing curl commands with Go's native http package. This makes the tests more efficient and the code more robust. My review focuses on the new HTTP client usage, where I've identified a resource leak and a few areas to improve error handling for easier debugging. The suggested changes will help ensure the new implementation is solid.
34be2da to
4abac63
Compare
|
/retest |
4abac63 to
e7b9371
Compare
e7b9371 to
002ed6e
Compare
| // The remote repo name is unique with a prefix of the local name. | ||
| func (b *BitbucketClient) CreateRepository(localName string) (string, error) { | ||
| u, err := uuid.NewRandom() | ||
| func (b *BitbucketClient) CreateRepository(name string) (string, error) { |
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.
I'd suggest that if the method no longer creates something it's better to pick a different name than just Create. Create should probably error out if something exists.
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.
Pull Request Overview
This PR refactors Bitbucket repository management to reuse repositories across test runs instead of creating and deleting them each time. Repository names are now derived from the test cluster name and RSync namespace/name, similar to other git providers. The implementation replaces curl-based API calls with Go's native http package for improved maintainability.
Key changes:
- Bitbucket repositories are now named deterministically and can be reused across test runs
- Repository deletion logic removed since repos persist and are reset between runs
- Replaced exec.Command curl calls with http.Client for better error handling
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| e2e/nomostest/gitproviders/bitbucket.go | Refactored to use http package, added repository name sanitization, removed deletion logic, and implemented repository reuse |
| e2e/nomostest/gitproviders/util/reponame.go | Added SanitizeBitbucketRepoName function with 62-char limit, extracted common sanitization logic, fixed double-dash issue |
| e2e/nomostest/gitproviders/util/reponame_test.go | Added tests for SanitizeBitbucketRepoName and verified max length constraints |
| e2e/nomostest/gitproviders/git-provider.go | Updated newBitbucketClient to accept clusterName parameter |
| e2e/nomostest/gitproviders/cloud_source_repository.go | Removed duplicate "cs-e2e-" prefix when calling SanitizeRepoName |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
002ed6e to
5c9721e
Compare
* Creates new repos based on the test cluster name and RSync namespace and name, similar to what is done by other gitproviders * Removes the deletion of Bitbucket repos since they can now be reused * Replaces the use of curl with the http package
5c9721e to
02cb348
Compare
|
/retest |
Uh oh!
There was an error while loading. Please reload this page.