Skip to content

Reuse installed ginkgo and use cleaner binary check - #609

Merged
chombium merged 1 commit into
cloudfoundry:mainfrom
jorbaum:minor-test-script-improvements
Aug 24, 2026
Merged

Reuse installed ginkgo and use cleaner binary check#609
chombium merged 1 commit into
cloudfoundry:mainfrom
jorbaum:minor-test-script-improvements

Conversation

@jorbaum

@jorbaum jorbaum commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

Description

Adjusts test scripts just like cloudfoundry/loggregator-agent-release#717 :

  • reuse ginkgo instead of recompiling every time, same as we are already doing with golangci-lint
  • cleaner check for binary removing three lines of code ;)
  • fix golangci-lint complaints

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Testing performed?

  • Unit tests
  • Integration tests
  • Acceptance tests

Checklist:

  • This PR is being made against the main branch, or relevant version branch
  • I have made corresponding changes to the documentation
  • I have added testing for my changes

@chombium chombium left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change the linting directives

Comment thread src/internal/auth/capi_client.go Outdated

func (c *CAPIClient) HasApp(sourceID, authToken string) bool {
req, err := http.NewRequest(http.MethodGet, c.addr+"/v3/apps/"+sourceID, nil)
req, err := http.NewRequest(http.MethodGet, c.addr+"/v3/apps/"+sourceID, nil) //#nosec G704

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use //nolint:gosec instead of #nosec as we are using gosec via golang and not directly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. There are several instances where there nosec is already used in this code base. Do you want me to change those as well?

@jorbaum jorbaum Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no more additions of nolint:gosec necessary. I adapted my PR. Also, in the meantime new code has been added using #gosec. I therefore do not think it is a good idea to move away from #nosec in this PR.

Comment thread src/internal/auth/capi_client.go Outdated
Comment thread src/internal/routing/static_lookup.go Outdated
Comment thread src/.golangci.yml Outdated
Comment thread src/.golangci.yml Outdated

@jorbaum jorbaum left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worked in the comments.

@jorbaum
jorbaum force-pushed the minor-test-script-improvements branch 4 times, most recently from e472868 to c5a872e Compare March 3, 2026 15:07
@jorbaum
jorbaum requested a review from chombium March 27, 2026 08:22
@jorbaum
jorbaum force-pushed the minor-test-script-improvements branch 2 times, most recently from ef4127b to d7c7db3 Compare August 24, 2026 12:34
Also:

* also fix lint issue with dualstack
@jorbaum
jorbaum force-pushed the minor-test-script-improvements branch from d7c7db3 to 4b5e31a Compare August 24, 2026 12:38
@github-project-automation github-project-automation Bot moved this from Inbox to Pending Merge | Prioritized in Application Runtime Platform Working Group Aug 24, 2026
@chombium
chombium merged commit ea5507e into cloudfoundry:main Aug 24, 2026
4 checks passed
@github-project-automation github-project-automation Bot moved this from Pending Merge | Prioritized to Done in Application Runtime Platform Working Group Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants