Skip to content

e2e: use regex replace to forcibly replace scaffolded Dockerfile base image strings#3874

Merged
joelanford merged 1 commit intooperator-framework:masterfrom
joelanford:e2e-dev-images
Sep 12, 2020
Merged

e2e: use regex replace to forcibly replace scaffolded Dockerfile base image strings#3874
joelanford merged 1 commit intooperator-framework:masterfrom
joelanford:e2e-dev-images

Conversation

@joelanford
Copy link
Copy Markdown
Member

Description of the change:
Use regex replacement to forcibly replace scaffolded Dockerfile base image strings

(Also ignore dot imports of ginkgo and gomega when linting)

Motivation for the change:
Follow-up from #3854 to ensure latest changes are tested in e2e tests

Checklist

If the pull request includes user-facing changes, extra documentation is required:

Copy link
Copy Markdown
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 12, 2020
Comment thread test/internal/utils.go
s := strings.Replace(string(b), old, new, -1)
s := matcher.ReplaceAllString(string(b), replace)
err = ioutil.WriteFile(path, []byte(s), info.Mode())
ExpectWithOffset(1, err).NotTo(HaveOccurred())
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.

we also should return an error when the content is not found to avoid this kind of scenario.
However, we can do it as follow up.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I went ahead and added a check to make sure a replacement actually occurs.

@openshift-ci-robot
Copy link
Copy Markdown

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 12, 2020
@joelanford
Copy link
Copy Markdown
Member Author

Ansible e2e tests were failing on the initial commit. I think it was because I tried to change all usages of ReplaceInFile to ReplaceRegexInFile. I've updated the PR to change just the Dockerfile version replacements to use ReplaceRegexInFile.

@joelanford joelanford merged commit 8e1bd94 into operator-framework:master Sep 12, 2020
@joelanford joelanford deleted the e2e-dev-images branch September 12, 2020 17:37
joelanford added a commit to joelanford/operator-sdk that referenced this pull request Sep 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants