GumTreeDiff / gumtree

An awesome code differencing tool
https://github.com/GumTreeDiff/gumtree/wiki
GNU Lesser General Public License v3.0
933 stars 174 forks source link

Build docker image on CI #324

Closed koppor closed 9 months ago

koppor commented 10 months ago

To enable refinement of the Docker image, this PR adds a build to the pipeline.

I think, it works, but internally, tests are failing. I adapted that test. I allowed two values, because the return values are varying:

 #13 136.2 TreeSitterTreeGeneratorsTest > testPython() FAILED
#13 136.2     org.opentest4j.AssertionFailedError: expected: <13> but was: <9>
#13 136.2         at app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
#13 136.2         at app//org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
#13 136.2         at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
#13 136.2         at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145)
#13 136.2         at app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:510)
#13 136.2         at app//com.github.gumtreediff.gen.treesitter.TreeSitterTreeGeneratorsTest.testPython(TreeSitterTreeGeneratorsTest.java:176)
 TreeSitterTreeGeneratorsTest > testPython() FAILED
    org.opentest4j.AssertionFailedError: expected: <9> but was: <13>
        at app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
        at app//org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145)
        at app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:510)
        at app//com.github.gumtreediff.gen.treesitter.TreeSitterTreeGeneratorsTest.testPython(TreeSitterTreeGeneratorsTest.java:176)

I also updated the checkout action to the most recent version.

jrfaller commented 10 months ago

Hi @koppor and thanks a lot for your interest in GumTree.

The test that breaks targets an external parser, I think it's not a big deal, maybe due to a checkout of a more recent / an older version.

If I understand correctly, the docker image is just built but not pushed anywhere, right? I am not sure it is feasible on GitHub but could we store/push this docker image somewhere? Right now it is periodically pushed to docker hub (by hand) and then used for the CI on this repo (container: gumtreediff/gumtree:latest) which is annoying.

Best regards.

koppor commented 10 months ago

If I understand correctly, the docker image is just built but not pushed anywhere, right?

Right!

I am not sure it is feasible on GitHub but could we store/push this docker image somewhere?

Yes. This is the "original" intention of the used GitHub action.

One could do periodically build+push on main and on tags.

Example available at https://github.com/docker/build-push-action#git-context

One needs a docker login name and a docker secret stored in the GitHub secrets.


This PR is a step in that direction. It will enable maintainers of this repository to work on it (because they have access to secrects)

koppor commented 9 months ago

I added the push feature. You need to configure two GitHub secrets:

The secrets are configured as follows:

  1. Open https://github.com/GumTreeDiff/gumtree/settings/secrets/actions
  2. At "Repository secrets" push the button "New repository secret"
  3. Add the key/value pair DOCKERHUB_USERNAME / your-user-name
  4. Repeat for 'DOCKERHUB_TOKEN`

The secrets are protected and cannot be accessed by non-contributors.

jrfaller commented 9 months ago

Awesome!!! I am wondering how often and on what kind of trigger we should do such a push, since I suspect it gonna take a long time right?

koppor commented 9 months ago

Depends on the definition of "a long time". Maybe 10 minutes? The push is done after all checks are green. I added a concurrency group. Thus, if there are multiple pushes to master shortly one after another, the latest one "wins" and cancels the ones before.

koppor commented 9 months ago

BTW, since the tag latest is offered at Dockerhub (https://hub.docker.com/r/gumtreediff/gumtree/tags), the expectation really is that "latest" points either to the latest release or the latest commit. I interpret it as the latest commit :p.

We could use "edge" for the latest commit on main and "latest" for the latest tag...

jrfaller commented 9 months ago

OK let's try ;-), merging this.

jrfaller commented 9 months ago

Seems like there is an issue : Error: Unable to locate executable file: docker. Please verify either the file path exists or the file can be found within a directory specified by the PATH environment variable. Also check the file mode to verify the file is executable., any idea? (https://github.com/GumTreeDiff/gumtree/actions/runs/7727264225)

koppor commented 9 months ago

I was too fast in merging the workflows:

container: gumtreediff/gumtree:latest

does not have docker installed. I will split the workflows again so that the normal github image is used for docker