🌱 Image name edge cases covered #3514
Conversation
k8s-ci-robot
added
cncf-cla: yes
test/e2e/utils/test_context.go
Outdated
| // Remove white space from image name | ||
| trimmedImageName := strings.Trim(t.ImageName, "") | ||
| // Check trimmedimage name is empty | ||
| if len(trimmedImageName) < 0 { |
There was a problem hiding this comment.
| if len(trimmedImageName) < 0 { | |
| if len(strings.Trim(t.ImageName, "")) < 0 { |
You can do in the same line and you do not need spent a memory space to store the data in a variable
Otherwise it is great !!!
There was a problem hiding this comment.
Can you please squash the commits for we are able to move forward?
@Sajiyah-Salat ?
There was a problem hiding this comment.
An important correction! Rest looks good!
Good work! @Sajiyah-Salat
test/e2e/utils/test_context.go
Outdated
| if _, err := t.Run(cmd); err != nil { | ||
| warnError(err) | ||
| // Check white space from image name | ||
| if len(strings.Trim(t.ImageName, "")) == 0 { |
There was a problem hiding this comment.
This should either be:
strings.Trim(t.ImageName, " ")or
strings.TrimSpace(t.ImageName)First option has an empty space in the second argument of the trim, which means we are intending to remove all whitespaces. The second one is a helper provided by golang itself: https://pkg.go.dev/strings#TrimSpace
Sajiyah-Salat
force-pushed
the
imagename_edgecase
branch
from
e9b35cc
to
c1ee572
Compare
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, Sajiyah-Salat The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
camilamacedo86
dismissed
varshaprasad96’s stale review
dismissing since the requested is addressed
|
Moving forward with this one since it is blocked by the preview of docs which is an issue that we are sorting out in another PR and has no relation with this one. |
camilamacedo86
merged commit 136aac6
into
kubernetes-sigs:master
imagename edge case cover
imagename edge case cover
feat: get data feat: add change feat: remove log feat: add e2e test build(deps): bump golang.org/x/tools from 0.11.0 to 0.12.0 Bumps [golang.org/x/tools](https://github.com/golang/tools) from 0.11.0 to 0.12.0. - [Release notes](https://github.com/golang/tools/releases) - [Commits](golang/tools@v0.11.0...v0.12.0) --- updated-dependencies: - dependency-name: golang.org/x/tools dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> chore(docs): change folder pkg to internal/controller in the migration guide fix: ensure external plugin can scaffold files in new dirs :seedling: Image name edge cases covered (kubernetes-sigs#3514) imagename edge case cover :seedling: logs info updated (kubernetes-sigs#3512) logs info updated :bug: fix doc preview 🐛: sync kb binary from source code for docs (kubernetes-sigs#3520) fix: sync kb binary from source code for docs :bug: fix typos in scaffolded makefiles :book: fix escaped html in markdown code segments :bug: (go/v4) fix makefile target to build images ensuring its single responsability refactor: externalize cfg :bug: (deployimage): fix e2e tests which are using go/v3 which is deprecated. Ensure that we are using the go/v4 ensure that we call go tidy when we generate sample docs :sparkles: (Kustomize/v2, go/v4): upgrade controller-tools from 0.12.0 to 0.13.0 (kubernetes-sigs#3561) (go/v4): Upgrade controller-tools from 0.12.0 to v0.13.0 :sparkles: improve logs from alpha command generate :bug: (deployimage/v1-beta1): fix scaffold with multigroup and add optional plugins to multigroup sample to validate changes :bug: change deprecated warning to yellow feat: remove quota fix: add space fix: change kind Update test/e2e/alphagenerate/generate_test.go Co-authored-by: Tony (TianYi) <kavinjsir@gmail.com>
Description:
In generating docs, if there is no image present we can skip running docker command and this was covered in #3508
but if the image name is of single character or in anyother edge case the code would crash.
In this pr I have covered the Edge cases.
Following up on: #3508 (comment)