fix: address a git time travel issue in the upgrade tests#85
Conversation
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! |
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>
| # Install the hub agent to the hub cluster. | ||
| kind export kubeconfig --name $HUB_CLUSTER | ||
| helm install hub-agent ../../charts/hub-agent/ \ | ||
| helm install hub-agent charts/hub-agent/ \ |
There was a problem hiding this comment.
how did this work if the dir is not correct?
There was a problem hiding this comment.
Hi Ryan! It was working because the setup part used to travel back in time so it runs using the old version, which involves steps that jump between the root dir and the /test/upgrade dir.
I realized last night it's a bit difficult to reason about, so this PR updates the behavior to become more consistent in the sense that we still leverage the old source code but all the upgrade test related scripts are from the latest commit.
There was a problem hiding this comment.
Pull Request Overview
This PR addresses a git time travel issue in the upgrade compatibility tests by updating the way the tests fetch and switch between different versions of the code. Key changes include:
- Removing explicit directory switching (-C "../..") for the Fleet agent docker builds and updating chart paths for helm commands.
- Updating workflow steps to check out the appropriate version of the test directory for before/after upgrade sequences.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/upgrade/upgrade.sh | Updated docker build and helm upgrade commands to use local paths. |
| test/upgrade/setup.sh | Updated helm install commands to reflect local chart paths. |
| .github/workflows/upgrade.yml | Changed workflow steps for version checkout and execution of test scripts. |
Comments suppressed due to low confidence (2)
test/upgrade/upgrade.sh:33
- The removal of the '-C "../.."' option changes the working directory context for the make command. Please verify that the build targets and working directory assumptions are still valid.
TAG=$IMAGE_TAG make docker-build-hub-agent
.github/workflows/upgrade.yml:86
- The variable '$PREVIOUS_COMMIT' is used in the workflow steps but does not appear to be defined in the current diff. Please ensure that it is properly assigned before using it to avoid unintended checkout behavior.
git checkout $PREVIOUS_COMMIT -- test/upgrade
Signed-off-by: michaelawyu <chenyu1@microsoft.com>
|
The E2E tests failed but it should not be related to this PR -> will merge it now to unblock the progress. If there's any concern, please let me know. |
…dev#85) * Minor fixes Signed-off-by: michaelawyu <chenyu1@microsoft.com> * Minor fixes Signed-off-by: michaelawyu <chenyu1@microsoft.com> * Minor fixes Signed-off-by: michaelawyu <chenyu1@microsoft.com> * Minor fixes Signed-off-by: michaelawyu <chenyu1@microsoft.com> * Minor fixes Signed-off-by: michaelawyu <chenyu1@microsoft.com> * Minor fixes Signed-off-by: michaelawyu <chenyu1@microsoft.com> * Minor fixes Signed-off-by: michaelawyu <chenyu1@microsoft.com> --------- Signed-off-by: michaelawyu <chenyu1@microsoft.com>
Description of your changes
This PR fixes a git time travel issue in the upgrade compatibility tests.
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