Skip to content
This repository has been archived by the owner on Mar 30, 2018. It is now read-only.

Add support for CI in a Windows x64 environment. #232

Merged
merged 33 commits into from May 12, 2017
Merged

Conversation

padraic
Copy link
Collaborator

@padraic padraic commented Apr 23, 2017

Appveyor is free to use, so long as we stay within the cache limits. I'm set this for an x64 env, assuming most people using PHP on Windows (don't cry!) have recent processors.

The initial run, assuming I haven't borked the configuration, is not passing with a few errors reported from PHPUnit.

Behat is not currently included in tests, but shouldn't be an issue to add later.

@padraic
Copy link
Collaborator Author

padraic commented Apr 23, 2017

Note: I granted access to the repository for the Appveyor service so it will show up on future commits. Bit of a nuisance as it will be in a failing state right now, but let's see if this something useful.

Copy link
Member

@theofidry theofidry left a comment

Choose a reason for hiding this comment

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

👍 some minor comments

appveyor.yml Outdated

test_script:
- cd c:\projects\workspace
- vendor/bin/phpunit -c phpunit.xml.dist
Copy link
Member

Choose a reason for hiding this comment

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

missing blank line

appveyor.yml Outdated
php_ver_target: 7.1

cache:
- '%LOCALAPPDATA%\Composer\files -> composer.lock'
Copy link
Member

Choose a reason for hiding this comment

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

do we need to cache all of that? I though '%LOCALAPPDATA%\Composer\files' was enough

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's not strictly necessary to cache anything other than composer, but Humbug is the only project likely to benefit from Windows testing in the org, so we have cache to burn on PHP installs.

@padraic
Copy link
Collaborator Author

padraic commented Apr 23, 2017

Damn, I was almost sure that Travis was missing in the checks. Silly Ubuntu scrollbars are too thin! I'd prefer to have styleci pushed to bottom as least critical.

@GrahamCampbell
Copy link
Contributor

Silly Ubuntu scrollbars are too thin!

Could be worse. On Chrome on Mac, there are no scrollbars. It only shows when i try to scroll it. :P

@GrahamCampbell
Copy link
Contributor

Would be cool if GitHub could group the pr and push statuses for the same service.

@padraic
Copy link
Collaborator Author

padraic commented Apr 23, 2017

@GrahamCampbell That would actually help. Of course, Travis could rebrand itself as APlusTravis or something and beat the alpha ordering ;).

commit 9035df0
Author: Pádraic Brady <padraic.brady@gmail.com>
Date:   Mon Apr 24 22:04:07 2017 +0100

    Use preset php target version

commit aab1904
Author: Pádraic Brady <padraic.brady@gmail.com>
Date:   Mon Apr 24 21:56:15 2017 +0100

    Make VC explicit

commit bf75d60
Author: Pádraic Brady <padraic.brady@gmail.com>
Date:   Mon Apr 24 21:43:25 2017 +0100

    Excl. 5.6

commit d31f302
Author: Pádraic Brady <padraic.brady@gmail.com>
Date:   Mon Apr 24 21:27:06 2017 +0100

    Work from c:\tools\php, and initiate web client

commit 8d79979
Author: Pádraic Brady <padraic.brady@gmail.com>
Date:   Mon Apr 24 21:13:24 2017 +0100

    Install xdebug via powershell using appropriate versions
@padraic
Copy link
Collaborator Author

padraic commented Apr 25, 2017

@humbug/core After tinkering (and a silly error on my part!), this should now be working.

Copy link
Member

@theofidry theofidry left a comment

Choose a reason for hiding this comment

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

Left some minor comments. 👍


environment:
matrix:
#- dependencies: lowest
Copy link
Member

Choose a reason for hiding this comment

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

I would add a comment linked to an issue or something here. Otherwise it's just a commented code/config we don't know if it can be removed or will be enabled later or something

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done with #235

return $path;
}
return sprintf('%s %s', $phpFinder->find(), $path);
/*
* Was: return sprintf('%s %s', $phpFinder->find(), $path);
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure to understand that comment either

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mainly just a memory aid. I assume there was some Windows variant where this actually worked, so that does crop up as an issue, might save some time debugging. For now through, the php command that was there really shouldn't be needed since the phpunit command is an executable script on Powershell.

@padraic
Copy link
Collaborator Author

padraic commented May 1, 2017

Changes today:

  • Fixed caching (barring any cache restore clashes with scripts)
  • Tried, but removed behat (the fails are mostly colour text related and need checking)
  • Skipped xdebug install if cached version is restored (creates a non-fatal warning).

@humbug/core Should be good to merge with one more review approval.

@theofidry
Copy link
Member

Maybe @pamil has an idea about those Behat failures? In any case I believe they can be fixed later. I would just add an issue to not forget it :)

Creates random fails on Appveyor where an update is not run
@padraic padraic merged commit da2a92c into master May 12, 2017
@padraic padraic deleted the ci/appveyor branch May 12, 2017 10:28
tabbabi pushed a commit to tabbabi/humbug that referenced this pull request Aug 4, 2017
tabbabi pushed a commit to tabbabi/humbug that referenced this pull request Aug 4, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants