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

tests trigger: add target-branch parameter to trigger from the right branch #4121

Merged
merged 4 commits into from
Jul 29, 2024

Conversation

aboedo
Copy link
Member

@aboedo aboedo commented Jul 26, 2024

Fix the @RCGitBot please test issue where tests would always get triggered from the main branch.

This solution is based on this comment: CircleCI-Public/trigger-circleci-pipeline-action#61 (comment)

But you can also see in the code that this is indeed how the branch name is determined, and that that logic looks the same as the one for GHA_action and target-slug, so it's just undocumented.

We're using a fixed version, 1.2.0, so it shouldn't just randomly break.

@aboedo aboedo added the ci label Jul 26, 2024
@aboedo aboedo requested a review from a team July 26, 2024 22:00
@aboedo aboedo self-assigned this Jul 26, 2024
@aboedo aboedo changed the title tests triggrer: add target-branch parameter to trigger from the right branch tests trigger: add target-branch parameter to trigger from the right branch Jul 26, 2024
# Even though it seems to be undocumented as of writing.

- name: 'Get GitHub branch'
run: echo ::set-output name=branch::$(gh pr view $PR_NO --repo $REPO --json headRefName --jq '.headRefName')
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess the gh CLI will be installed right? I would imagine so, being a github action 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

it is, listed here:
https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2204-Readme.md#cli-tools

I'll add a comment that this assumes that it's installed, though, just in case it randomly gets removed one day, although I can't imagine that would ever be the case

@@ -31,12 +31,27 @@ jobs:
fi
echo "User is a member of the organization"


Copy link
Contributor

Choose a reason for hiding this comment

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

super nit but there's an extra line here

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

Copy link
Member Author

Choose a reason for hiding this comment

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

updated!

Copy link
Member

@JayShortway JayShortway left a comment

Choose a reason for hiding this comment

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

Nice job getting this fixed! 🙌

# But we can also see that the following code reads from this parameter: https://github.com/CircleCI-Public/trigger-circleci-pipeline-action/blob/a81cd720792a6088debd7f182b552845abb86f1b/src/lib/CircleCIPipelineTrigger.ts#L66
# Even though it seems to be undocumented as of writing.

- name: 'Get GitHub branch'
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: maybe this step can be named "Get PR branch"? "PR" is the relevant bit imo.

Copy link
Member Author

Choose a reason for hiding this comment

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

great call, updated!

@aboedo aboedo enabled auto-merge (squash) July 29, 2024 12:59
@aboedo aboedo merged commit 8a476d6 into main Jul 29, 2024
3 of 4 checks passed
@aboedo aboedo deleted the andy/fix-trigger-tests-from-comment-branch branch July 29, 2024 13:03
jamesrb1 pushed a commit that referenced this pull request Aug 7, 2024
…branch (#4121)

Fix the `@RCGitBot please test` issue where tests would always get
triggered from the `main` branch.

This solution is based on this comment:
CircleCI-Public/trigger-circleci-pipeline-action#61 (comment)

But you can also see in the code that this is indeed how the branch name
is determined, and that that [logic looks the same as the one for
`GHA_action` and
`target-slug`](https://github.com/CircleCI-Public/trigger-circleci-pipeline-action/blob/a81cd720792a6088debd7f182b552845abb86f1b/src/lib/CircleCIPipelineTrigger.ts#L66),
so it's just undocumented.

We're using a fixed version, 1.2.0, so it shouldn't just randomly break.
MarkVillacampa pushed a commit that referenced this pull request Aug 7, 2024
**This is an automatic release.**

### Bugfixes
* Fix Paywalls crash on iOS 18 beta (#4154) via Andy Boedo (@aboedo)
### Dependency Updates
* Bump danger from 9.4.3 to 9.5.0 (#4143) via dependabot[bot]
(@dependabot[bot])
* Bump nokogiri from 1.16.6 to 1.16.7 (#4129) via dependabot[bot]
(@dependabot[bot])
* Bump fastlane from 2.221.1 to 2.222.0 (#4130) via dependabot[bot]
(@dependabot[bot])
### Other Changes
* Update deployment targets for tests (#4145) via Andy Boedo (@aboedo)
* Deploy purchaserTester: clean up dry-run parameter (#4140) via Andy
Boedo (@aboedo)
* Clean up API Testers (#4141) via Andy Boedo (@aboedo)
* More project structure cleanup (#4131) via Andy Boedo (@aboedo)
* temporarily disables purchasetester deploy (#4133) via Andy Boedo
(@aboedo)
* Fix trigger all tests branch (#4135) via Andy Boedo (@aboedo)
* Clean up XCWorkspace and testing apps (#4111) via Andy Boedo (@aboedo)
* tests trigger: add target-branch parameter to trigger from the right
branch (#4121) via Andy Boedo (@aboedo)
* Re-added the RevenueCatUI tests job on every commit (#4113) via Andy
Boedo (@aboedo)
nyeu pushed a commit that referenced this pull request Oct 2, 2024
…branch (#4121)

Fix the `@RCGitBot please test` issue where tests would always get
triggered from the `main` branch.

This solution is based on this comment:
CircleCI-Public/trigger-circleci-pipeline-action#61 (comment)

But you can also see in the code that this is indeed how the branch name
is determined, and that that [logic looks the same as the one for
`GHA_action` and
`target-slug`](https://github.com/CircleCI-Public/trigger-circleci-pipeline-action/blob/a81cd720792a6088debd7f182b552845abb86f1b/src/lib/CircleCIPipelineTrigger.ts#L66),
so it's just undocumented.

We're using a fixed version, 1.2.0, so it shouldn't just randomly break.
nyeu pushed a commit that referenced this pull request Oct 2, 2024
**This is an automatic release.**

### Bugfixes
* Fix Paywalls crash on iOS 18 beta (#4154) via Andy Boedo (@aboedo)
### Dependency Updates
* Bump danger from 9.4.3 to 9.5.0 (#4143) via dependabot[bot]
(@dependabot[bot])
* Bump nokogiri from 1.16.6 to 1.16.7 (#4129) via dependabot[bot]
(@dependabot[bot])
* Bump fastlane from 2.221.1 to 2.222.0 (#4130) via dependabot[bot]
(@dependabot[bot])
### Other Changes
* Update deployment targets for tests (#4145) via Andy Boedo (@aboedo)
* Deploy purchaserTester: clean up dry-run parameter (#4140) via Andy
Boedo (@aboedo)
* Clean up API Testers (#4141) via Andy Boedo (@aboedo)
* More project structure cleanup (#4131) via Andy Boedo (@aboedo)
* temporarily disables purchasetester deploy (#4133) via Andy Boedo
(@aboedo)
* Fix trigger all tests branch (#4135) via Andy Boedo (@aboedo)
* Clean up XCWorkspace and testing apps (#4111) via Andy Boedo (@aboedo)
* tests trigger: add target-branch parameter to trigger from the right
branch (#4121) via Andy Boedo (@aboedo)
* Re-added the RevenueCatUI tests job on every commit (#4113) via Andy
Boedo (@aboedo)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants