Skip to content

make env handling more robust #76

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

BradleyGoulding
Copy link

This is a potential fix for #75

It filters the --env-file process to ensure it only passes valid formatted env values

@g105b
Copy link
Member

g105b commented May 16, 2025

Hey thanks! I have an example repository at https://github.com/php-actions/example-phpunit that I will make a test for at some point soon. Feel free to create an example usage as a PR on that repo if you can do it before me.

@BradleyGoulding
Copy link
Author

BradleyGoulding commented May 19, 2025

Sure, i will have a look, I need to create a negative test, so I will set an invalid env var, and with the current php-actions/composer@v6, and php-actions/phpunit@v4, this will fail. if i use my forks they will pass.

You can see the behaviour here: https://github.com/BradleyGoulding/example-phpunit/actions

Do you want me to just open the PR with the test that sets the environment up, pointing to current php-actions/composer@v6, and php-actions/phpunit@master, or wait till you merge the patch and update to include that?

My concern is that merging in an intentionally broken test pending the patch, will harm other work.

@g105b
Copy link
Member

g105b commented May 20, 2025

I have an idea - please create a PR, but I'll merge it into its own branch instead of master, otherwise I agree that things might become a bit confusing. I'll document the branch in the readme to explain what it's testing.

Thank you for your work on this. I'm a bit too busy with work projects to be able to pay too much attention to it at the moment.

@BradleyGoulding
Copy link
Author

I have opened the PR php-actions/example-phpunit#15, this adds the workflow that validates this issue. As this impacts multiple repos, let me know if you need me to do anything further.

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

Successfully merging this pull request may close these issues.

2 participants