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

Automatically Run Tests on PR and Push Docker Image #38

Merged
merged 27 commits into from
Oct 19, 2022

Conversation

withinboredom
Copy link
Collaborator

@withinboredom withinboredom commented Oct 18, 2022

This uses GitHub Actions to automatically run the tests in Docker via go test and publish successful builds to a Docker registry:

  • PR's are pushed to :pr-#, e.g., frankenphp:pr-38 (subject to actions approval by maintainer)
  • main branch is pushed to :main
  • tags are pushed to latest and if using semver, corresponding tags, e.g., tag v1.2.3 is tagged :1.2, :1.2.3, and latest

See the Docker metadata action for more information.

Requires several repo secrets:

  • REGISTRY_LOGIN_SERVER: The URL to push the docker image to.
  • REGISTRY_USERNAME: The username/org to login as.
  • REGISTRY_PASSWORD: The password to login with.
  • REGISTRY_REPO: The repository to push the docker image to.

What is built

Note that this builds amd64 and arm64 images and the first time this is built per branch takes ~2.5 hours.

Documentation

I've added some documentation about how this works, but I kinda just made it up. So we can change it here, you can change it after we merge it, or we can delete it in this PR to be added later.

@withinboredom withinboredom marked this pull request as draft October 18, 2022 16:32
@dunglas
Copy link
Owner

dunglas commented Oct 19, 2022

This looks great. Should I do something else than setting the tokens?

@withinboredom withinboredom changed the title Automatically Run Tests on PR Automatically Run Tests on PR and Push Docker Image Oct 19, 2022
@withinboredom withinboredom marked this pull request as ready for review October 19, 2022 16:57
@withinboredom
Copy link
Collaborator Author

withinboredom commented Oct 19, 2022

This looks great. Should I do something else than setting the tokens?

Yep @dunglas, you should just need to set the secrets. IIRC, there might be a button right around the merge button that allows this PR to build in GitHub actions. It also might be buried in a setting somewhere. Otherwise, you'll have to merge it to see if it works.

@withinboredom
Copy link
Collaborator Author

withinboredom commented Oct 19, 2022

I think everything is working well and this is ready for review. There are some test images built on my Docker Hub using these changes. I have some ideas to greatly decrease the build time, but it'll be a while before I have the time to get to it (essentially create the base image in your fork of php-src instead of here).

I've also basically made up some release documentation, but we can delete it, change it, or do whatever you feel is appropriate. I mostly put it there to document how to use these changes.


COPY . .
COPY *.* .
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I changed this to *.* because my editor likes to put things in a dotfile, which caused the cache to always blow up.

labels: ${{ steps.meta.outputs.labels }}
builder: ${{ steps.buildx.outputs.name }}
cache-from: type=gha
cache-to: type=gha,mode=max
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll add a new line here.

labels: ${{ steps.meta.outputs.labels }}
builder: ${{ steps.buildx.outputs.name }}
cache-from: type=gha
cache-to: type=gha,mode=max
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll add a new line here.

@dunglas
Copy link
Owner

dunglas commented Oct 19, 2022

essentially create the base image in your fork of php-src instead of here

Don't bother with that, I think that I'll manage to make FrankenPHP compatible with upstream 8.2 soon.

@dunglas dunglas merged commit c158887 into dunglas:main Oct 19, 2022
@dunglas
Copy link
Owner

dunglas commented Oct 19, 2022

Thank you!

@woonoz-ac woonoz-ac mentioned this pull request Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants