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

Remove AppVeyor, run E2E tests during CI #1365

Merged
merged 1 commit into from Oct 24, 2020

Conversation

sanmai
Copy link
Member

@sanmai sanmai commented Oct 22, 2020

This PR:

  • Runs integration and end-to-end tests on Windows using GitHub Actions.
  • Adds a separate mutation testing workflow, with only PHP 7.4/PCOV and Linux/Windows
  • Disables AppVeyor (will report success all the time)
  • Runs full set of E2E tests on Ubuntu, without a .phar yet.

The one bit I cannot do: protected branch config needs new Windows checks to set required. Also, IIRC version branches are not protected, they might need some love too.

There's no need to drop a requirement for AppVeyor: it is still used by earlier branches.

@sanmai sanmai marked this pull request as ready for review October 22, 2020 14:25
 - Use absolute path for PHPUnit.
 - Use bash as default shell in Makefile.
 - Run a subset of E2E tests on Windows.
 - Without BENCHMARK_SOURCES yet
 - Add a sane memory-limit
 - Update InitialConfigBuilderTest
 - Add a dedicated mutation testing workflow, with only PHP 7.4/PCOV and Linux/Windows
@sanmai sanmai added the DX Developer Experience label Oct 23, 2020
@sanmai sanmai mentioned this pull request Oct 23, 2020
6 tasks
@@ -22,7 +23,7 @@ PHP_CS_FIXER_CACHE=build/cache/.php_cs.cache

PHPSTAN=./vendor/bin/phpstan

PHPUNIT=vendor/bin/phpunit
PHPUNIT=vendor/phpunit/phpunit/phpunit
Copy link
Member Author

Choose a reason for hiding this comment

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

This path works both under Linux and on Windows.

@@ -1,3 +1,4 @@
SHELL=bash
Copy link
Member Author

Choose a reason for hiding this comment

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

This isn't the default for Windows, therefore we have to be explicit here.

Makefile Outdated Show resolved Hide resolved
run: |
php bin/infection -j2 --test-framework-options="--group=default" --ignore-msi-with-no-mutations
sudo apt-get install -y --no-install-recommends expect
Copy link
Member Author

Choose a reason for hiding this comment

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

Getting Expect on Windows, is, well, complicated.

This could be worked around with WSL, but it'll make Windows VM "unpure": we can't expect everyone using Infection with Windows with WSL.

shell: bash
run: |
ls tests/e2e/*/composer.json | xargs dirname |
xargs -I{} composer --working-dir={} update --no-interaction --prefer-dist --no-progress ${{ matrix.dependencies }}
Copy link
Member Author

Choose a reason for hiding this comment

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

Surprise! This works on Windows too!

tests/phpunit/Logger/ConsoleLoggerTest.php Outdated Show resolved Hide resolved
if: runner.os == 'Windows'
shell: bash
run: |
make test-e2e-phpunit PHP='phpdbg -qrr' E2E_PHPUNIT_GROUP=e2e BENCHMARK_SOURCES=
Copy link
Member Author

Choose a reason for hiding this comment

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

It just wouldn't work with Xdebug. It seems installed, but then it is not.

restore-keys: |
composer-${{ runner.os }}-${{ matrix.php-version }}-${{ hashFiles('**/composer.*') }}-
composer-${{ runner.os }}-${{ matrix.php-version }}-${{ hashFiles('composer.*') }}-
Copy link
Member Author

Choose a reason for hiding this comment

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

We only need to consider two files here, no others in tests/e2e or otherwise.

e2e-vendor-${{ runner.os }}-${{ matrix.php-version }}-${{ hashFiles('tests/e2e/*/composer.json') }}-
e2e-vendor-${{ runner.os }}-${{ matrix.php-version }}-
e2e-vendor-${{ runner.os }}-
composer-
Copy link
Member Author

Choose a reason for hiding this comment

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

Installing anything with Composer is insanely slow in Windows, just compare these 4 seconds on Linux with these 34 seconds on Windows. The solution is to cache the dependencies, but not only archives, the whole vendor folders. Note that we don't cache composer.lock here: it takes far less time to verify that all is in order.

This might be a mistake, but let's see.

@@ -39,6 +40,8 @@ BENCHMARK_SOURCES=tests/benchmark/MutationGenerator/sources \
tests/benchmark/Tracing/coverage \
tests/benchmark/Tracing/sources

E2E_PHPUNIT_GROUP=integration,e2e
PHPUNIT_GROUP=default
Copy link
Member Author

Choose a reason for hiding this comment

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

To let us override these from outside.

fi

diff -u -w expected-output.txt infection.log
}
Copy link
Member Author

Choose a reason for hiding this comment

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

SymfonyFlex needs a special environment variable, and it uses old version of PHPUnit which is not compatible with the one we use, so we can't really run this test from E2ETest. Adding this file prevents this test from running from E2ETest, and also solves env variable issue.

if (PHP_EOL === "\r\n") {
$this->markTestSkipped('Test fixture uses Unix line endings');
}

Copy link
Member Author

Choose a reason for hiding this comment

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

We haven't run this test on Windows before, apparently.


matrix:
fast_finish: true
clone_depth: 1
Copy link
Member

Choose a reason for hiding this comment

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

why do we need to keep this file?

Copy link
Member Author

Choose a reason for hiding this comment

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

AppVeyor will fail the build without it.

Copy link
Member

Choose a reason for hiding this comment

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

so will we remove once 0.18 is not maintained anymore?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's the idea. Remove it once we don't need it anywhere.

@maks-rafalko
Copy link
Member

The one bit I cannot do: protected branch config needs new Windows checks to set required. Also, IIRC version branches are not protected, they might need some love too.

let's merge and I will set it up

@sanmai sanmai merged commit 4353f98 into infection:master Oct 24, 2020
@sanmai sanmai deleted the pr/2020-10/no-appveyor branch October 24, 2020 08:33
maks-rafalko pushed a commit to maks-rafalko/infection that referenced this pull request Oct 24, 2020
 - Use absolute path for PHPUnit.
 - Use bash as default shell in Makefile.
 - Run a subset of E2E tests on Windows.
 - Without BENCHMARK_SOURCES yet
 - Add a sane memory-limit
 - Update InitialConfigBuilderTest
 - Add a dedicated mutation testing workflow, with only PHP 7.4/PCOV and Linux/Windows
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Developer Experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants