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 composer create-project test to GitHub Actions #115

Merged
merged 1 commit into from
May 19, 2023

Conversation

zonuexe
Copy link
Contributor

@zonuexe zonuexe commented Jan 6, 2023

Q A
Documentation no
Bugfix no
BC Break no
New Feature no
RFC no
QA yes

Description

refs #114 (comment)

Add a GitHub Action that reproduces the behavior of composer create-project.

@zonuexe zonuexe force-pushed the feature/ci-install-archived branch from cacbe8e to b9a0edb Compare January 6, 2023 17:31
@zonuexe zonuexe changed the title Add composer create-project test to GitHub Actions Add composer create-project test to GitHub Actions Jan 6, 2023
@zonuexe zonuexe force-pushed the feature/ci-install-archived branch 5 times, most recently from 4c3cbc0 to 43788a0 Compare January 6, 2023 17:57
with:
php: ${{ matrix.php_version }}
- name: Checkout sourcecode
uses: actions/checkout@v3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

laminas/laminas-ci-matrix-action says An actions/checkout step prior to this action is not required, but requires an explicit checkout action to execute the command in the next step.

Signed-off-by: USAMI Kenta <tadsan@zonu.me>
@zonuexe zonuexe force-pushed the feature/ci-install-archived branch from 43788a0 to 6342704 Compare January 6, 2023 18:04
run: |
cd ..
yes 1 | composer create-project mezzio/mezzio-skeleton test-new-project \
--repository='{"type": "path", "url": "./mezzio-skeleton"}' --stability=dev
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left a way to reproduce the behavior using a git archive in #114 (comment), but I found that this way I could test it using a real composer create-project command.

This time the project is created based on the local path instead of the version published on Packagist, which can be verified by truncating the file path in OptionalPackages::preparePhpunitConfig().

スクリーンショット 2023-01-07 3 12 21

For some reason composer creates test-new-project as a symlink instead of creating a new directory, which is fine for a working check.

matrix:
php_version:
- '@latest'
- '@lowest'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test cannot rely on the matrices provided by laminas-continuous-integration-action, so it only checks @lowest and @latest.

@zonuexe
Copy link
Contributor Author

zonuexe commented Jan 6, 2023

@Ocramius This PR is ready, please review it.

Copy link
Contributor

@weierophinney weierophinney left a comment

Choose a reason for hiding this comment

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

I was unsure why you were using the CI action, and then realized it will auto-identify the lowest and highest supported PHP versions, as well as any extensions required, ensuring the composer operation will run.

The only "issue" I see is that it doesn't demonstrate selecting different packages when prompted. That could be an improvement for a later revision, though.

One other thing that might be interesting is having some assertions on completion: validating configuration is correct, etc.

However, I think this is a great addition, and I'm happy to 🚢 now, and iterate and improve from here!

Copy link
Member

@boesing boesing left a comment

Choose a reason for hiding this comment

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

LGTM.
Not sure why the CI action is added, even tho it would install extensions (within the CI container), it has no effect to the runner where you run the create-project command.

What PHP versions are @lowest and @latest pointing to in GHA? Not sure if it is a good idea to use these instead of the supported PHP versions from within the composer.json as they might change over time while the composer.json might not.

However, good enough for me.

Copy link
Member

@gsteel gsteel left a comment

Choose a reason for hiding this comment

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

LGTM! However, might it be easier here to drop the laminas matrix action and use https://github.com/WyriHaximus/github-action-composer-php-versions-in-range to generate a matrix of PHP versions to check?

@zonuexe
Copy link
Contributor Author

zonuexe commented Jan 9, 2023

It is possible to replace that action. However, rather than adding a dependency on third-party Actions, I believe that Laminas community-maintained Action is more reliable.

@boesing
Copy link
Member

boesing commented Jan 9, 2023

It is possible to replace that action. However, rather than adding a dependency on third-party Actions, I believe that Laminas community-maintained Action is more reliable.

I understand your concern, but using laminas-ci-action just boots the CI container while not executing anything in there. So its actually just like sleep(10); exit 0; and brings no value to the current setup you are providing with this PR.

Why did you add the CI action in the first place? Maybe if I understand the reason, we can provide a better fix.

BTW: We as laminas do not really care if something is provided from us or from a trustworthy upstream project. IMHO, the wyrhaximus GHA has proper reputation and can be used properly as the user is working with PHP for ages now 👍🏼


Just to clarify: https://github.com/mezzio/mezzio-skeleton/actions/runs/3857405715/jobs/6574764223#step:3:8
Thats the step where the CI container is being executed and it says "nothing to execute" as there is no JOB JSON payload passed to the container. The create-project is not executed in the laminas-ci container but on the GHA runner. Therefore, you can safely remove laminas-ci-container as a step and the GHA output would remain the same.

You can also add php --version right before you run composer create-project. This will help to debug which PHP version is being used when you are expecting @lowest or @latest (I still do not know what these aliases are pointing to from a GHA PoV).

Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

🚢 here

Better done good than done perfect 👍

@Ocramius Ocramius merged commit 65de3d5 into mezzio:3.14.x May 19, 2023
@Ocramius Ocramius self-assigned this May 19, 2023
@Ocramius
Copy link
Member

Fun stuff: this failed post-merge 🤣

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants