Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@nturgut
Copy link
Contributor

@nturgut nturgut commented Apr 28, 2020

use $CIRRUS_BASE_BRANCH for getting the branch name when cloning the flutter repo.

Successful run for the failing integration test: https://cirrus-ci.com/task/6347234272870400

Fixes flutter/flutter#55884

use $CIRRUS_BASE_BRANCH for getting the branch name when cloning the flutter repo.
@nturgut nturgut requested a review from pcsosinski April 28, 2020 21:51
@auto-assign auto-assign bot requested a review from iskakaushik April 28, 2020 21:52
@nturgut nturgut requested review from yjbanov and removed request for iskakaushik April 28, 2020 21:52
Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

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

RSLGTM


# Special handling of release branches.
ENGINE_BRANCH_NAME=`git branch | grep '*' | cut -d ' ' -f2`
ENGINE_BRANCH_NAME=$CIRRUS_BASE_BRANCH
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this variable? Can we just use CIRRUS_BASE_BRANCH?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In summary when we wrote clone_flutter.sh script, we though git branch will fetch the corresponding branch name and we will change the branch if, test is running on a release branch. however we didn't see it in actions since all tests so far was on master. with the new release we realize git branch return (no branch). That is why web integration tests were failing for the release. so I started using this cirrus ci parameter. I tested a few more variables such as $BRANCH $CIRRUS_BRANCH only this one returns the value we need.

Copy link
Contributor

Choose a reason for hiding this comment

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

What I mean is if you remove this line then search and replace ENGINE_BRANCH_NAME with CIRRUS_BASE_BRANCH, wouldn't the result be the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, sorry I didn't get your question! as you will see I use ENGINE_BRANCH_NAME in a lot of places. I also think is a more descriptive variable name than CIRRUS_BRANCH_NAME. We can also keep ENGINE_BRANCH_NAME when we are using this script in LUCI too.

@nturgut
Copy link
Contributor Author

nturgut commented Apr 29, 2020

Merging the PR. Thanks for the review!

@nturgut nturgut merged commit 2db3276 into master Apr 29, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 29, 2020
pcsosinski pushed a commit to pcsosinski/engine that referenced this pull request May 1, 2020
use $CIRRUS_BASE_BRANCH for getting the branch name when cloning the flutter repo.
pcsosinski pushed a commit that referenced this pull request May 2, 2020
* use $CIRRUS_BASE_BRANCH for the branch name (#18014)

use $CIRRUS_BASE_BRANCH for getting the branch name when cloning the flutter repo.

* Change the repo fetch script used in integration tests (#17943)

* change the repo fetch script to recognize candidate versions such as flutter-1.17-candidate.3. Originally the script only accepted branches such as v0.7.3 as valid engine branches.

* addressing reviewer comments: changing the release regular expression

Co-authored-by: Nurhan Turgut <[email protected]>
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 5, 2020
@chinmaygarde chinmaygarde deleted the nturgut-clone-flutter-fix branch November 3, 2021 21:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[web] web_integration_tests cirrus task are failing for flutter-1.17-candidate.3

3 participants