-
Notifications
You must be signed in to change notification settings - Fork 6k
use a different flag to detect release branch for post submit tests #18655
use a different flag to detect release branch for post submit tests #18655
Conversation
|
I haven't created a PR for a release branch before, so I'm not sure if this is the right approach. Please let me know if I'm missing something. |
|
thanks! is this a change on the branch or a cherrypick from master? if the former, what happens the next time we branch? |
|
You should generally land this on master and then cherry-pick it here. Alternatively, given the difficulty in testing this, if you want to land it and test it here first, then land it on master once it's verified to work, that's ok with me. |
| # During the commit tests the base branch value is empty, instead | ||
| # `CIRRUS_BRANCH` has the correct branch name. | ||
| ENGINE_BRANCH_NAME=$CIRRUS_BASE_BRANCH | ||
| if [[ -z $CIRRUS_BASE_BRANCH ]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to surround $CIRRUS_BASE_BRANCH with quotes
| # On presubmit, we can get the branch name from the `CIRRUS_BASE_BRANCH` flag. | ||
| # During the commit tests the base branch value is empty, instead | ||
| # `CIRRUS_BRANCH` has the correct branch name. | ||
| ENGINE_BRANCH_NAME=$CIRRUS_BASE_BRANCH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$CIRRUS_BASE_BRANCH should be surrounded with quotes here too
| # `CIRRUS_BRANCH` has the correct branch name. | ||
| ENGINE_BRANCH_NAME=$CIRRUS_BASE_BRANCH | ||
| if [[ -z $CIRRUS_BASE_BRANCH ]] | ||
| then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: typically then goes on the same line as the if in bash
| if [[ -z $CIRRUS_BASE_BRANCH ]] | ||
| then | ||
| echo "Running post commit tests use CIRRUS_BRANCH instead." | ||
| ENGINE_BRANCH_NAME=$CIRRUS_BRANCH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surround with quotes
|
Colsing this PR now. I'll address these changes in a PR to master. Since integration tests also run on LUCI now, I also need to address the branch/release issue on LUCI. |
Using CIRRUS_BRANCH flag for getting the engine branch, for post-submit tests. (CIRRUS_BASE_FLAG is empty on post-submit. it is used for testing PRs presubmits though)