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

Run tests workflows on all commits of a PR #142

Closed
wojsmol opened this issue Oct 11, 2022 · 17 comments
Closed

Run tests workflows on all commits of a PR #142

wojsmol opened this issue Oct 11, 2022 · 17 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@wojsmol
Copy link

wojsmol commented Oct 11, 2022

Is your feature request related to a problem? Please describe.
Run tests workflows on commits generated in push_to_branch() function.
Describe the solution you'd like
Add a possibility to run tests workflows on commits generated in push_to_branch() function. Now this is blocked because github.actor is equal to github-actions[bot] witch is excluded from running additional workflows.

Describe alternatives you've considered
None

Additional context
First commit of PR created using create_pull_request() function is ok and tests are run. Problem can be observed for example here joomla/core-translations#477

@wojsmol wojsmol added the enhancement New feature or request label Oct 11, 2022
@andrii-bodnar andrii-bodnar added help wanted Extra attention is needed hacktoberfest This issue welcomes contributions for Hacktoberfest labels Oct 11, 2022
@andrii-bodnar andrii-bodnar removed the hacktoberfest This issue welcomes contributions for Hacktoberfest label Nov 25, 2022
@wojsmol
Copy link
Author

wojsmol commented Dec 6, 2022

Hi
Any progress on this?

@andrii-bodnar
Copy link
Member

Hi @wojsmol, are there any alternative solutions you've considered?

@wojsmol
Copy link
Author

wojsmol commented Dec 7, 2022

@andrii-bodnar I'm considering to disable automatic PR creation by this action and use it only to download translations from crowdin and then create PR for example using github cli, but it will be nice to fix that in that action.

I wonder if correct Github token is used in push_to_branch function because we are using custom PAT to run tests on crowdin PR's - you can see that here https://github.com/joomla/core-translations/blob/08afd1f5e4802ea696f3a6ccdbb0d3ce1f7636db/.github/workflows/crowdin-v4-dl-package-translations.yml#L22-L43.

@zomars
Copy link

zomars commented Jan 12, 2023

We're having the same issue. This is related to #34

@wojsmol
Copy link
Author

wojsmol commented Oct 10, 2023

@andrii-bodnar Any progress on resolving this issue?

@andrii-bodnar
Copy link
Member

andrii-bodnar commented Oct 11, 2023

@wojsmol probably not. Actually, I have no clear idea how to fix it. We use the user-defined GITHUB_TOKEN env variable for authentication. Should we also allow passing the GITHUB_ACTOR env variable to the GH Action?

@wojsmol
Copy link
Author

wojsmol commented Oct 11, 2023

@andrii-bodnar Personally I was thinking about moving

github-action/entrypoint.sh

Lines 210 to 212 in f749b05

echo "CONFIGURATION GIT USER"
git config --global user.email "${INPUT_GITHUB_USER_EMAIL}"
git config --global user.name "${INPUT_GITHUB_USER_NAME}"
before
REPO_URL="https://${GITHUB_ACTOR}:${GITHUB_TOKEN}@${INPUT_GITHUB_BASE_URL}/${GITHUB_REPOSITORY}.git"
so user is defined before we use ${GITHUB_ACTOR} in REPO_URL but this is hard for me to test.

@andrii-bodnar
Copy link
Member

@wojsmol I just pushed the suggested fix to a separate branch - 7ebe921

Could you please try it to see if it works for you? You can specify the commit hash as the action version:

uses: crowdin/github-action@7ebe921dc710cc29debc4deea5716864a8342c65

@wojsmol
Copy link
Author

wojsmol commented Oct 13, 2023

@andrii-bodnar PR created joomla/core-translations#783 after merge we will wail until next crowdin created PR witch more then one commit.

@wojsmol
Copy link
Author

wojsmol commented Oct 13, 2023

@andrii-bodnar As we can see on joomla/core-translations#784 this change don't fix the issue.

@wojsmol
Copy link
Author

wojsmol commented Oct 13, 2023

@andrii-bodnar Looking further in to this function we can try to skip --no-verify here

github-action/entrypoint.sh

Lines 231 to 233 in f749b05

echo "PUSH TO BRANCH ${BRANCH}"
git commit --no-verify -m "${INPUT_COMMIT_MESSAGE}"
git push --no-verify --force "${REPO_URL}"
if that will work then --no-verify should be possible to remove after passing additional input.

@andrii-bodnar
Copy link
Member

@wojsmol I'm not sure if I get you right. Do you mean removing the --no-verify parameter for the git commit and git push commands?

@wojsmol
Copy link
Author

wojsmol commented Oct 16, 2023

@andrii-bodnar yes

@andrii-bodnar
Copy link
Member

andrii-bodnar commented Oct 18, 2023

@wojsmol just pushed the change to the test branch. Could you please try the following version hash?

uses: crowdin/github-action@8a8d8c3944421259eebb52975e0a3fee672ed638

@wojsmol
Copy link
Author

wojsmol commented Oct 18, 2023

@andrii-bodnar PR created and referenced

@wojsmol
Copy link
Author

wojsmol commented Oct 21, 2023

@andrii-bodnar After further testing on my side I find out that tests are running on all commits on stable crowdin github action version after adding custom PAT to checkout action like this

    - name: Checkout
      uses: actions/checkout@v4
      with:
        token: ${{ secrets.GHA_CUSTOM_PAT }}

custom PAT is also passed into each crowdin github action invocation. This will be nice as a example.
PR with tests on all commits joomla/core-translations#805

@andrii-bodnar
Copy link
Member

@wojsmol thanks a lot for finding the solution! 🚀 🎉

Already added it to the Action's Wiki - Run tests workflows on all commits of a PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants