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 appveyor.yml for C.I. on Windows #15575

Merged
merged 2 commits into from Aug 26, 2015
Merged

Conversation

nicolas-grekas
Copy link
Member

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? YES, both on Travis (Linux) and on Appveyor (Windows)!
Fixed tickets #13934, #15049, #14259, #15045, #15444
License MIT
Doc PR symfony/symfony-docs#5654
  • testing two matrix lines:
    • one without mbtring nor fileinfo nor intl
    • one with these ext enables, intl version 51.2 so that almost no test is skipped on our Intl component
  • bug fixes thanks to these harder testing conditions
  • some display bug on appveyor, reported here.

@stof
Copy link
Member

stof commented Aug 18, 2015

should we also try compiling the debug extension on Windows ? Or should we wait until we decide whether we move it to a separate repo ?
In any case, it is irrelevant for 2.3 as it does not have the C extension

- php composer.phar install --prefer-dist --no-interaction
test_script:
- cd C:\projects\symfony
- php -r "readfile('https://phar.phpunit.de/phpunit.phar');" | php
Copy link
Member

Choose a reason for hiding this comment

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

what about using native ways to download files instead ? http://www.appveyor.com/docs/how-to/download-file

Copy link
Member

Choose a reason for hiding this comment

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

btw, this is already done for the php.zip file.

and downloading phpunit should also be part of the installation rather than the tests

Copy link
Member Author

Choose a reason for hiding this comment

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

Because I'd prefer having commands read/writables by us. And also because cakephp does this way :)

Copy link
Member

Choose a reason for hiding this comment

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

but the current comment could have issues. see https://github.com/symfony/symfony-installer/issues/137

@nicolas-grekas
Copy link
Member Author

That would be great to have the debug ext compiled also, but let's first make this one work.

@fabpot
Copy link
Member

fabpot commented Aug 18, 2015

Account has been created now. Try to push here to trigger a new build as I don't seem to have any way to do that from the interface.

@nicolas-grekas
Copy link
Member Author

@fabpot thanks, did you setup the webhook?
Here is a helper screenshot for PR support:

11

- echo extension=php_mbstring.dll >> php.ini
- echo extension=php_fileinfo.dll >> php.ini
- php -r "readfile('https://getcomposer.org/installer');" | php
- echo @php c:\php\composer.phar %*>composer.bat
Copy link
Member

Choose a reason for hiding this comment

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

your composer bat file does not work

@nicolas-grekas
Copy link
Member Author

The build is now working! But phpunit crashes before going to the end of the test suite :(
Any Windows expert here?

- php c:\php\composer.phar install --prefer-dist --no-progress
- mkdir phpunit
- cd phpunit
- ps: Start-FileDownload 'https://github.com/sebastianbergmann/phpunit/archive/4.7.zip'
Copy link
Member

Choose a reason for hiding this comment

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

the current stable version of PHPUnit is 4.8, not 4.7

@stof
Copy link
Member

stof commented Aug 18, 2015

@nicolas-grekas can you try running the testsuite in smaller chunks (component by component) to see whether it helps ? We are not running the full testsuite in 1 go on Travis (even though the main goal was to parallelize runs to be faster)

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Aug 18, 2015 via email

@nicolas-grekas
Copy link
Member Author

Any PowerShell scripter here? We would need a ps script to run phpunit tests in parallel, the same as we do on unixes with find src/Symfony -mindepth 3 -type f -name phpunit.xml.dist -printf '%h\n' | parallel --gnu --keep-order 'echo -e "\\nRunning {} tests"; php --exclude-group tty,benchmark,intl-data {} || (echo "KO {}" && $(exit 1));'

Invole-Parallel could maybe help, but I must admit I'm a total noob here.

@nicolas-grekas
Copy link
Member Author

An other idea could be to run bash from cygwin, it's maybe even already somewhere on the vm.

@wouterj
Copy link
Member

wouterj commented Aug 18, 2015

@nicolas-grekas I have both powershell and cygwin on my PC, what do you want me to do/try?

@nicolas-grekas
Copy link
Member Author

Status: needs review
Except for a transient test on the Process component, tests are green.

@@ -38,11 +38,16 @@ public function testConstructorDefaultTimeZone()
$formatter = $this->getDateFormatter('en', IntlDateFormatter::MEDIUM, IntlDateFormatter::SHORT);

// In PHP 5.5 default timezone depends on `date_default_timezone_get()` method
if (PHP_VERSION_ID >= 50500) {
if (PHP_VERSION_ID >= 50500 || (extension_loaded('intl') && method_exists('IntlDateFormatter', 'setTimeZone'))) {
Copy link
Member Author

Choose a reason for hiding this comment

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

pecl intl 3.0 backports php5.5 behavior to 5.3/5.4

@@ -568,8 +570,16 @@ public function setTimeZoneId($timeZoneId)

try {
$this->dateTimeZone = new \DateTimeZone($timeZoneId);
if ('GMT' !== $timeZoneId && $this->dateTimeZone->getName() !== $timeZoneId) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Some invalid timezones are accepted by DateTimeZone since 5.5.10, but rejected by IntlDateFormatter.

@nicolas-grekas
Copy link
Member Author

ping @symfony/deciders , especially @webmozart

@fabpot
Copy link
Member

fabpot commented Aug 26, 2015

Not mergeable yet as there is still a failing test on Windows.

@fabpot
Copy link
Member

fabpot commented Aug 26, 2015

Thank you @nicolas-grekas.

@fabpot fabpot merged commit ea5d656 into symfony:2.3 Aug 26, 2015
fabpot added a commit that referenced this pull request Aug 26, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

Add appveyor.yml for C.I. on Windows

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | YES, both on Travis (Linux) and on Appveyor (Windows)!
| Fixed tickets | #13934, #15049, #14259, #15045, #15444
| License       | MIT
| Doc PR        | symfony/symfony-docs#5654

- testing two matrix lines:
  - one without mbtring nor fileinfo nor intl
  - one with these ext enables, intl version 51.2 so that almost no test is skipped on our Intl component
- bug fixes thanks to these harder testing conditions
- some display bug on appveyor, [reported here](http://help.appveyor.com/discussions/suggestions/197-support-ansi-color-codes).

Commits
-------

ea5d656 Windows and Intl fixes
8bbd8d9 Add appveyor.yml for C.I. on Windows
install:
- IF EXIST c:\php (SET PHP=0) ELSE (mkdir c:\php)
- cd c:\php
- IF %PHP%==1 appveyor DownloadFile http://windows.php.net/downloads/releases/archives/php-5.4.43-nts-Win32-VC9-x86.zip
Copy link
Member

Choose a reason for hiding this comment

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

@nicolas-grekas why testing against PHP 5.4 which is nearly EOL rather than against PHP 5.6 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

because I couldn't find a .dll for intl + icu 51.2 for 5.5 (vc9-nts-x86)

@nicolas-grekas nicolas-grekas deleted the appveyor branch August 26, 2015 11:50
xabbuh added a commit to symfony/symfony-docs that referenced this pull request Sep 8, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

Doc about new way of running tests

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | yes
| Applies to    | all
| Fixed tickets | -

Related to symfony/symfony#15575

Commits
-------

dc754c8 Doc about new way of running tests
This was referenced Oct 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants