test: set the upgrade compatibility test pipeline to use commit-specific setup#64
Conversation
Signed-off-by: michaelawyu <chenyu1@microsoft.com>
Signed-off-by: michaelawyu <chenyu1@microsoft.com>
Signed-off-by: michaelawyu <chenyu1@microsoft.com>
Signed-off-by: michaelawyu <chenyu1@microsoft.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the upgrade compatibility test pipeline to use commit-specific setups by removing in-script git checkouts and handling version switching entirely in the GitHub Actions workflow.
- Removed internal
GIT_TAGand branch/commit checkout logic fromupgrade.shandsetup.sh. - Added explicit “travel back” and “return to current state” steps in
.github/workflows/upgrade.yml. - Updated
setup.shheader comment and pipeline invocation (introduced a couple of typos in echo statements).
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/upgrade/upgrade.sh | Removed git-tag checkout and branch-restore logic |
| test/upgrade/setup.sh | Removed checkout logic, added header note (typos in some echo lines) |
| .github/workflows/upgrade.yml | Added pre-/post-upgrade git checkout steps for commit-specific setup |
| run: | | ||
| go install github.com/onsi/ginkgo/v2/ginkgo@v2.19.1 | ||
|
|
||
| - name: Travel back in time to the before upgrade version |
There was a problem hiding this comment.
[nitpick] This git checkout block is repeated for each upgrade scenario; consider extracting it into a reusable composite action or workflow step to reduce duplication.
There was a problem hiding this comment.
Pull Request Overview
This PR updates the upgrade compatibility test pipeline to use commit-specific setup by removing the in-script Git checkout logic and shifting that responsibility into the workflow.
- Removed commit checkout logic in upgrade.sh
- Removed commit-specific restore logic and updated messaging in setup.sh
- Updated GitHub Actions workflow to handle commit switching for both before and after upgrade scenarios
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/upgrade/upgrade.sh | Removed Git checkout logic to use commit-specific setups externally |
| test/upgrade/setup.sh | Removed previous branch/commit restoration and updated log messages |
| .github/workflows/upgrade.yml | Added explicit steps to switch to the desired commit/tag and back |
| - name: Travel back in time to the before upgrade version | ||
| run: | | ||
| GIT_TAG="${{ github.event.inputs.beforeTagOrCommit }}" | ||
| PREVIOUS_BRANCH=$(git branch --show-current) |
There was a problem hiding this comment.
[nitpick] The logic to capture the current branch and commit is repeated across multiple workflow steps. Consider extracting this repetitive logic into a reusable script or GitHub Action to improve maintainability.
There was a problem hiding this comment.
Second this, there are a lot of duplicated code here
There was a problem hiding this comment.
Hi Ryan! This part is a bit tricky unfortunately. The commands manipulate the current state of the repo; as a script file it would mean that once traveling back the script would be gone (I could add a step that copies out the script as a untracked file, but that would be a bit weird I guess -> but if it is OK with you I could make the changes); for the separate GitHub Action proposal, it would require a separate public repo dedicated to the Action as required by GitHub, which is also tricky.
| # baseline commit, i.e., the commit that triggers the workflow. | ||
| run: | | ||
| echo "Returning to the current state..." | ||
| git checkout $PREVIOUS_COMMIT |
There was a problem hiding this comment.
why is this not current commit?
There was a problem hiding this comment.
Hi Ryan! The upgrade pipeline often runs on dangling commits (commits that have not been checked as the PR hasn't been merged) so we couldn't just jump to the HEAD.
| - name: Travel back in time to the before upgrade version | ||
| run: | | ||
| GIT_TAG="${{ github.event.inputs.beforeTagOrCommit }}" | ||
| PREVIOUS_BRANCH=$(git branch --show-current) |
There was a problem hiding this comment.
Second this, there are a lot of duplicated code here
Signed-off-by: michaelawyu <chenyu1@microsoft.com>
…o feat/upgrade-test-commit-match Signed-off-by: michaelawyu <chenyu1@microsoft.com>
Description of your changes
This PR sets the upgrade compatibility test pipeline to use commit-specific setup (i.e., use old helm chart with old image).
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested
N/A
Special notes for your reviewer
N/A