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

Ini handling broken when passed via output parameter #405

Closed
2 of 5 tasks
jrfnl opened this issue Feb 4, 2021 · 3 comments
Closed
2 of 5 tasks

Ini handling broken when passed via output parameter #405

jrfnl opened this issue Feb 4, 2021 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@jrfnl
Copy link
Contributor

jrfnl commented Feb 4, 2021

First, let me please thank you for this action! I think it's awesome and very much appreciate all the work on it!

Describe the bug
I think this is related to the fix which was committed in response to issue #392.

Short version:

Setting ini parameters in a separate step and passing them to the setup-php action using step.stepid.outputs.outputname no longer works.

Long version:

For a project which has to support a large number of PHP versions as well as a large range of versions for a "framework", some PHP/framework combinations will generate PHP deprecation notices.
These do not break the actual product, but do cause a lot of noise in the output of the unit tests, so when running the tests via GH Actions against older versions of the "framework", error_reporting is set to E_ALL & ~E_DEPRECATED, while when testing against the current version of the framework, it is set to E_ALL.

As this needs some logic, the ini-value string is created in a separate step and saved as an output parameter. This output parameter is subsequently used in the next step to call setup-php.

This was working fine until about a month ago (last good run), but started to fail about three weeks ago.

The failing tests were all of the Failed asserting that exception of type "PHPUnit\Framework\Error\Warning" is thrown. kind and could not be reproduced locally, no matter what PHP/framework/PHPUnit combinations were used.

After various test runs and debug trial & error, I can only conclude that it is related to how the ini-values are handled by the setup-php action.

Version

  • I have checked releases, and the bug exists in the latest patch version of v1 or v2.
  • v2
  • v1

Runners

  • GitHub Hosted
  • Self Hosted

Operating systems
ubuntu-latest

PHP versions
PHP 5.4 up to 8.1

To Reproduce

This is the relevant part of the script:

      - name: Setup ini config
        id: set_ini
        run: |
          # On stable PHPCS versions, allow for PHP deprecation notices.
          # Unit tests don't need to fail on those for stable releases where those issues won't get fixed anymore.
          # Also set the "short_open_tag" ini to make sure specific conditions are tested.
          if [ ${{ matrix.custom_ini }} == "true" ]; then
            if [ "${{ matrix.phpcs_version }}" != "dev-master" ]; then
              echo '::set-output name=PHP_INI::error_reporting=E_ALL & ~E_DEPRECATED, short_open_tag=On'
            else
              echo '::set-output name=PHP_INI::error_reporting=E_ALL, short_open_tag=On'
            fi
          else
            if [ "${{ matrix.phpcs_version }}" != "dev-master" ]; then
              echo '::set-output name=PHP_INI::error_reporting=E_ALL & ~E_DEPRECATED'
            else
              echo '::set-output name=PHP_INI::error_reporting=E_ALL'
            fi
          fi

      - name: Install PHP
        uses: shivammathur/setup-php@v2
        with:
          php-version: ${{ matrix.php }}
          ini-values: ${{ steps.set_ini.outputs.PHP_INI }}
          coverage: none

The full script can be found here: https://github.com/PHPCompatibility/PHPCompatibility/blob/43b2b058b7875b249661ce23c62402c72067fa64/.github/workflows/test.yml

Expected behavior
That the ini-values are handled correctly, allowing for the tests to pass.

Screenshots/Logs
See links above in the description for relevant action logs.

Are you willing to submit a PR?
Would love to, but have too much on my plate right now and the hours spend trying to figure out what was going wrong here didn't help.... 😒

@jrfnl jrfnl added the bug Something isn't working label Feb 4, 2021
jrfnl added a commit to PHPCompatibility/PHPCompatibility that referenced this issue Feb 4, 2021
A recent change in the `shivammathur/setup-php@v2` has broken the handling of ini values passed via step output parameters.

I have [reported this bug upstream](shivammathur/setup-php#405).

For now, I'm fixing the build failures by commenting out the custom ini value setting.

Once the upstream bug has been fixed, this commit should be reverted.
@shivammathur
Copy link
Owner

@jrfnl
Sorry about this, I missed accounting for error constants. It should not quote them now.
Fixed in 1686147

Let me know if the issue is still there.

@jrfnl
Copy link
Contributor Author

jrfnl commented Feb 4, 2021

@shivammathur Thanks for this really quick turn-around on the issue! I'll start a build on our default branch now to confirm the fix.

@jrfnl
Copy link
Contributor Author

jrfnl commented Feb 4, 2021

The builds are turning green again. Fix confirmed. Thanks so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants