Skip to content
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

Windows tests should not checkout main branch and merge the PR branch #6175

Closed
rm3l opened this issue Sep 27, 2022 · 2 comments · Fixed by #6177
Closed

Windows tests should not checkout main branch and merge the PR branch #6175

rm3l opened this issue Sep 27, 2022 · 2 comments · Fixed by #6177
Assignees
Labels
area/testing Issues or PRs related to testing, Quality Assurance or Quality Engineering area/Windows Issues or PRs specific to Windows kind/bug Categorizes issue or PR as related to a bug.
Milestone

Comments

@rm3l
Copy link
Member

rm3l commented Sep 27, 2022

/kind bug
/area testing
/area Windows

What versions of software are you using?

Output of odo version:

odo v3.0.0-rc2 (f78096d26)

Server: https://10.10.10.105:6443
Kubernetes: v1.24.6

Actual behavior

Currently, Windows tests checkout the main branch and then merge the PR branch into the local main branch:

Shout "Checkout to $GIT_PR_NUMBER"
git fetch -v origin pull/${GIT_PR_NUMBER}/head:pr${GIT_PR_NUMBER}
git checkout main
git merge pr${GIT_PR_NUMBER} --no-edit

This can lead to potential issues for several reasons:

  • it does not guarantee that the code tested is the one from the PR branch. At the time the test is run, if the PR branch is behind main, with main containing changes not conflicting with the PR branch, the merge operation would succeed. But this would give a local branch with changes coming from both main and the PR branch. I think this explains the behavior I observed and reported in check if namespace is created instead of project #6108 (comment) (where a given test spec had run, but the PR branch had no reference to that test spec) and which led me to file Display Git commit ID in output of odo commands where the version is shown #6131 (for better traceability).
  • At the time the test is run, if there are conflicts between main and the PR branch, the whole Windows test would not pass.
  • This might prevent from testing PRs targeting branches other than main, like release/v3.0.0.

To me, the test should run with what is exactly in the PR branch, and not attempt to merge anything (or if this intended, it should be a dedicated stage in the Pipeline). We already have the OpenShift CI bot reporting potential conflicts and requesting to rebase PRs.

Expected behavior

Just like what is done in Kubernetes and OpenShift tests, Windows tests should rely on the code already checked out by IBM Cloud.
We could scp the current folder to the Windows machine, as we do with the PowerShell script: https://github.com/redhat-developer/odo/blob/main/.ibm/pipelines/windows-test.sh#L13.
Or if the scp operation is too slow, the Windows test script could perform a shallow checkout of the right PR branch (rather than merging it into the local main branch).

@openshift-ci openshift-ci bot added kind/bug Categorizes issue or PR as related to a bug. area/testing Issues or PRs related to testing, Quality Assurance or Quality Engineering area/Windows Issues or PRs specific to Windows labels Sep 27, 2022
@rm3l rm3l linked a pull request Sep 30, 2022 that will close this issue
3 tasks
@rm3l rm3l added this to the v3.0.0 🚀 milestone Sep 30, 2022
@rm3l rm3l moved this to In Review 👀 in odo Project Sep 30, 2022
@rm3l rm3l moved this to For Review in odo v3.0.0 Oct 1, 2022
@rm3l
Copy link
Member Author

rm3l commented Oct 3, 2022

Fixed by #6177 (and merged into release/v3.0.0). This will get merged into main right after the v3.0.0 tag.

/close

@openshift-ci openshift-ci bot closed this as completed Oct 3, 2022
@openshift-ci
Copy link

openshift-ci bot commented Oct 3, 2022

@rm3l: Closing this issue.

In response to this:

Fixed by #6177, which will get merged into main right after the v3.0.0 tag.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Repository owner moved this from For Review to Done in odo v3.0.0 Oct 3, 2022
Repository owner moved this from In Review 👀 to Done ✅ in odo Project Oct 3, 2022
@rm3l rm3l added the v3 label Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing Issues or PRs related to testing, Quality Assurance or Quality Engineering area/Windows Issues or PRs specific to Windows kind/bug Categorizes issue or PR as related to a bug.
Projects
Archived in project
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants