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

GH Actions: run tests also on Windows OS #170

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Mar 27, 2024

When running the tests locally, I realized that patch #146 did not actually work correctly on Windows.

This commit adds test runs against Windows in CI on a limited number of PHP versions to prevent this kind of issue going unnoticed for future PRs.

Note: the lowest PHP version I can get a running build on is PHP 5.5. This is related to SSL transport issues with Packagist with old Composer versions (which are needed for old PHP versions).


⚠️ The above mentioned bug should be fixed & merged first before this PR can get a passing build.

@jrfnl jrfnl added this to the 2.0.0 milestone Mar 27, 2024
@jrfnl jrfnl force-pushed the feature/ghactions-run-tests-against-windows branch 3 times, most recently from ff2bd4f to dffe775 Compare March 27, 2024 17:50
@jrfnl jrfnl force-pushed the feature/ghactions-run-tests-against-windows branch from dffe775 to 2dfde55 Compare April 10, 2024 12:50
@staabm
Copy link

staabm commented Oct 1, 2024

I tried debugging this problem on my windows machine and this did not lead to anything useful.
I feel its a weird bug in php-src, which happens on windows because of running skip-linting.php via proc_open.

Opened a php-src bug with my observations so far

@cmb69
Copy link

cmb69 commented Oct 1, 2024

$this->assertSame(0, $result->getCheckedFilesCount());

looks wrong to me; shouldn't the expectation be 1?

@staabm
Copy link

staabm commented Oct 1, 2024

thanks for looking into it.

looks wrong to me; shouldn't the expectation be 1?

as I understood the test, expectations are:

  • we try to lint example.php
  • since example.php contains a <?php // lint < 5.3 header and we are running with PHP >= 5.3, it should be considered "skipped"

-> thats why

        $this->assertSame(0, $result->getCheckedFilesCount());
        $this->assertSame(0, $result->getFilesWithSyntaxErrorCount());
        $this->assertSame(1, $result->getSkippedFilesCount());

look correct to me.

because of the problems described in php/php-src#16147 I think parallel-lint via skip-linting.php does not see the "skip-condition" // lint < 5.3 and therefore the test fails

@cmb69
Copy link

cmb69 commented Oct 1, 2024

Indeed, the expectation is apparently correct. Sorry for the noise.

When running the tests locally, I realized that patch 146 did not actually work correctly on Windows.

This commit adds test runs against Windows in CI on a limited number of PHP versions to prevent this kind of issue going unnoticed for future PRs.

Note: the lowest PHP version I can get a running build on is PHP 5.5. This is related to SSL transport issues with Packagist with old Composer versions (which are needed for old PHP versions).
@jrfnl jrfnl force-pushed the feature/ghactions-run-tests-against-windows branch from 2dfde55 to 8becf57 Compare October 2, 2024 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants