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 initial appveyor file #121

Merged
merged 11 commits into from
Mar 22, 2017
Merged

Add initial appveyor file #121

merged 11 commits into from
Mar 22, 2017

Conversation

orf
Copy link
Contributor

@orf orf commented Mar 21, 2017

Refs #120

@orf
Copy link
Contributor Author

orf commented Mar 21, 2017

Hmm, this should be starting a build now...

@cfarrow
Copy link
Contributor

cfarrow commented Mar 21, 2017

I'm looking into it.

@cfarrow
Copy link
Contributor

cfarrow commented Mar 21, 2017

It ran. I might have the integration wrong at the github end.

screen shot 2017-03-21 at 11 57 41 am

@cfarrow
Copy link
Contributor

cfarrow commented Mar 21, 2017

It looks like notifications are configured in the appveyor.yaml file. https://www.appveyor.com/docs/notifications/#github-pull-request

@cfarrow
Copy link
Contributor

cfarrow commented Mar 21, 2017

No, I'm misunderstanding something. That build is for master and notifications only need to be configured for custom templates. Still looking...

@orf
Copy link
Contributor Author

orf commented Mar 21, 2017

I think that is only for the comment, for example in this appveyor.yml file there are no notifications, but merge requests still get the notification like so: ember-cli/ember-cli#6708

@cfarrow
Copy link
Contributor

cfarrow commented Mar 21, 2017

I've enabled the webhook. I don't see a way to trigger a branch build from the appveyor interface. Please push another commit.

@orf
Copy link
Contributor Author

orf commented Mar 21, 2017

That's the ticket! Awesome, I'll have a fiddle tomorrow and see if we can get at least the installation rather than the building passing.

@cfarrow
Copy link
Contributor

cfarrow commented Mar 21, 2017

Cool. Thanks for taking the initiative on this!

@orf
Copy link
Contributor Author

orf commented Mar 21, 2017

No problem! It seems comtypes uses a VS 2010 build, but the minimum VS that appveyor offers is 2013. Is there any technical reason the project couldn't be updated to 2013?

@cfarrow
Copy link
Contributor

cfarrow commented Mar 21, 2017

No part of the comtypes library actually needs VS. It's there to build a COM server that is used to test that functionality.

@orf
Copy link
Contributor Author

orf commented Mar 22, 2017

Hmm, my test was simply running setup.py install - should it be building the test COM server in that case? I've changed it to a pip install so lets see if that helps.

@orf
Copy link
Contributor Author

orf commented Mar 22, 2017

Oh, my bad, it appears that AppVeyor triggers the build automatically.

@orf
Copy link
Contributor Author

orf commented Mar 22, 2017

Hey @cfarrow - this MR is ready for merging if you're happy (you might want to let github squash it when you do).

I tried to get the tests to run as well but failed. I think they are somewhat broken and need a lot of work to get them running, but for now just running setup.py install on the various python versions is a win.

So as I see it the tests appear to run on Python 2.7 32bit only only. 2.6 runs, but fails due to a missing testcase method (https://ci.appveyor.com/project/cfarrow/comtypes/build/1.0.9/job/6uxqxeleqe7j0hgi). All 3.x cases fail with an AttributeError (https://ci.appveyor.com/project/cfarrow/comtypes/build/1.0.9/job/i528o3q1kwvqdt1v).

It seems the tests in general are in a poor state, with a custom test loader that seems pretty old. I'd quite like to convert them to pytest if you're willing?

@cfarrow
Copy link
Contributor

cfarrow commented Mar 22, 2017

We can merge this as-is. I may record the test failures as their own issues, depending on how long they persist. I've moved the pytest discussion to #123.

@cfarrow
Copy link
Contributor

cfarrow commented Mar 22, 2017

Thanks for setting this up!

@cfarrow cfarrow merged commit b448218 into enthought:master Mar 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants