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

Add GH action for determining the commit range #287

Merged
merged 9 commits into from
Feb 17, 2022
Merged

Conversation

akaihola
Copy link
Owner

@akaihola akaihola commented Feb 16, 2022

Add a revision: parameter for the GitHub Action. If omitted, defaults to comparing HEAD to

  • the branch point where the current branch was created (for pull requests), or
  • the commit which the previous workflow job was run for.

The commit-range action implementation is from tue-robotics/tue-env/ci/commit-range/action.yml
Copyright (c) 2022, Eindhoven University of Technology - CST Robotics Group
Licensed under the BSD 3-Clause License.
See .github/actions/commit-range/LICENSE.md

Fixes #260

@akaihola akaihola added the CI label Feb 16, 2022
@akaihola akaihola added this to the 1.4.1 milestone Feb 16, 2022
@akaihola akaihola self-assigned this Feb 16, 2022
@akaihola
Copy link
Owner Author

akaihola commented Feb 16, 2022

@MatthijsBurgh, does my implementation look correct?
What about the description above, would that be usable for documentation as well?
Do you have a suitable repository to test this in?

@akaihola
Copy link
Owner Author

Also @MatthijsBurgh, are you aware of a good way to have automated tests for GitHub actions? Do we need a separate dummy repository to run it in? We could of course have unit tests for the main.py script in this repository as well.

Copy link
Collaborator

@yoursvivek yoursvivek left a comment

Choose a reason for hiding this comment

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

LGTM

@akaihola
Copy link
Owner Author

Thanks @yoursvivek. I haven't had the time to actually try this out for real though. Definitely need to do that.

@akaihola
Copy link
Owner Author

I'm trying this out in akaihola/ipython_pytest#4. Got this error:

Download action repository 'akaihola/darker@gh-action-revisions' (SHA:79befb86f49919588c3fc963721a2d8b52b91712)
Error: akaihola/darker/gh-action-revisions/action.yml:
Error: akaihola/darker/gh-action-revisions/action.yml: (Line: 22, Col: 7, Idx: 652) - (Line: 22, Col: 45, Idx: 690): While parsing a block mapping, did not find expected key.
Error: System.ArgumentException: Unexpected type '' encountered while reading 'action manifest root'. The type 'MappingToken' was expected.
   at GitHub.DistributedTask.ObjectTemplating.Tokens.TemplateTokenExtensions.AssertMapping(TemplateToken value, String objectDescription)
   at GitHub.Runner.Worker.ActionManifestManager.Load(IExecutionContext executionContext, String manifestFile)
Error: Fail to load akaihola/darker/gh-action-revisions/action.yml

@akaihola akaihola force-pushed the gh-action-revisions branch from 5a5c149 to 8dd88ac Compare February 17, 2022 20:58
The action implementation is from
https://github.com/tue-robotics/tue-env/blob/a37ee191d4a3b73f354d43b1623831040b58a90b/ci/commit-range/action.yml
Copyright (c) 2022, Eindhoven University of Technology - CST Robotics Group
Licensed under the BSD 3-Clause License.
See `.github/actions/commit-range/LICENSE.md`
@akaihola akaihola force-pushed the gh-action-revisions branch from 8dd88ac to 4912986 Compare February 17, 2022 21:00
@akaihola
Copy link
Owner Author

akaihola commented Feb 17, 2022

I fixed the above problem in action.yml. I also added a YAML linter workflow and formatted all YAML files in the repository accordingly.

I was also able to fix this problem which occurred in akaihola/ipython-pytest#4:

Run akaihola/darker@gh-action-revisions
  with:
    src: .
    options: --check --diff
    version: 1.4.0
Error: Can't find 'action.yml', 'action.yaml' or 'Dockerfile'
       under '/home/runner/work/ipython_pytest/ipython_pytest/.github/actions/commit-range'.
       Did you forget to run actions/checkout before running your local action?

The solution is to refer to the commit-range action with both the Darker repository name and the action directory path instead of just a local action directory path.

But one question remains: how to ensure commit-range is cloned and executed from the same branch as akaihola/darker?
I guess the obvious solution is to include the @1.4.1 suffix in uses: akaihola/darker/.github/actions/[email protected] and update it on each release.

Updating version numbers is ok, since there are already several occurrences of the version number and I'm certainly going to automate updating of those everywhere (in a way similar to what bump2version does – I just haven't yet found an off-the-shelf solution which would have the necessary features and no flaws).

@akaihola akaihola force-pushed the gh-action-revisions branch from 0f9b7b0 to 362d20a Compare February 17, 2022 21:47
@akaihola akaihola merged commit 5bd4e97 into master Feb 17, 2022
@akaihola akaihola deleted the gh-action-revisions branch February 17, 2022 21:48
@akaihola
Copy link
Owner Author

@MatthijsBurgh this is now released in 1.4.1.

Could you check whether everything went correctly in akaihola/ipython_pytest#4?

In build #5 (triggered by push), the commit range is empty:

   env:
    commit_list: [
    {
      "author": {
        "email": "[email protected]",
        "name": "Antti Kaihola",
        "username": "akaihola"
      },
      "committer": {
        "email": "[email protected]",
        "name": "Antti Kaihola",
        "username": "akaihola"
      },
      "distinct": true,
      "id": "d8af955248f594aa0e8c63424459ba46d177f939",
      "message": "Update Darker Action to version 1.4.1",
      "timestamp": "2022-02-18T00:20:13+02:00",
      "tree_id": "fab60c74fd14f1602e15ba07136b333aa4173f87",
      "url": "https://github.com/akaihola/ipython_pytest/commit/d8af955248f594aa0e8c63424459ba46d177f939"
    }
  ]
 COMMIT_RANGE =  

Is this correct?

@MatthijsBurgh
Copy link
Collaborator

Not this probably not correct.

The default value of os.getenv is only used if it isn't set. Not if is set, but the value resolves to False. I got the same problem. See the action_debug branch for debug and the fix.

@MatthijsBurgh
Copy link
Collaborator

What about the description above, would that be usable for documentation as well?

I don't know what you are referring to.

Do you have a suitable repository to test this in?

I am running this in https://github.com/MatthijsBurgh/epydoc. The test_darker branch should fail on the PR test on linting.

@MatthijsBurgh
Copy link
Collaborator

Also @MatthijsBurgh, are you aware of a good way to have automated tests for GitHub actions? Do we need a separate dummy repository to run it in? We could of course have unit tests for the main.py script in this repository as well.

I have no experience on that. I don't automatically test my GH actions. Depending on the type of test it could be in this repo or a separate one.

@akaihola
Copy link
Owner Author

@MatthijsBurgh,

Thanks for working on the fix in the action_debug branch. Looks good and clean.

By suitability of the description to the documentation I meant that we should probably explain what the default commit range is if the user doesn't override the revision: parameter. I was wondering whether the description of this issue #287 explains it accurately enough and could be adapted to the README:

Add a revision: parameter for the GitHub Action. If omitted, defaults to comparing HEAD to

  • the branch point where the current branch was created (for pull requests), or
  • the commit which the previous workflow job was run for.

As for automated tests, I'm not super enthusiastic right now to dive into researching how to do it, so maybe we'll just do manual testing for now and hope we won't need to do a lot of changes to the Action.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

Darker GH action checks against HEAD commit -> no diff
3 participants