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

Build docker image on CI #324

Merged
merged 5 commits into from
Jan 31, 2024
Merged

Build docker image on CI #324

merged 5 commits into from
Jan 31, 2024

Conversation

koppor
Copy link
Contributor

@koppor koppor commented Jan 12, 2024

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 jrfaller self-requested a review January 17, 2024 14:13
@jrfaller jrfaller self-assigned this Jan 17, 2024
@jrfaller
Copy link
Member

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
Copy link
Contributor Author

koppor commented Jan 22, 2024

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
Copy link
Contributor Author

koppor commented Jan 31, 2024

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
Copy link
Member

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
Copy link
Contributor Author

koppor commented Jan 31, 2024

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
Copy link
Contributor Author

koppor commented Jan 31, 2024

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
Copy link
Member

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

@jrfaller jrfaller merged commit 2b3fc26 into GumTreeDiff:main Jan 31, 2024
1 check failed
@jrfaller
Copy link
Member

jrfaller commented Jan 31, 2024

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
Copy link
Contributor Author

koppor commented Jan 31, 2024

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

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

Successfully merging this pull request may close these issues.

2 participants