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

Adding appveyor testing and opting in ci-helpers #273

Merged
merged 5 commits into from
Jan 10, 2016

Conversation

bsipocz
Copy link
Member

@bsipocz bsipocz commented Jan 8, 2016

This is to address and close #259.

I've also added ccdproc as a project on appveyor itself for astropy.

@hamogu
Copy link
Member

hamogu commented Jan 8, 2016

The Travis test fails, because the pytest plugin pytest-capture log is not installed. Before your PR, that was done manually in .travis.yml, now you rely on pytest_requires in setup.py, which does not work.
See: astropy/astropy-helpers#212

Either pip pytest-capture log need to be called by hand (in .travis.yml) or we need to add this package to install_requires (although technically it's not required for the installation, just to run the tests).

Note that I have a strong interest to make this PR work, too, because my own PRs #274 and #275 now have failing tests because the appveyor setup is not complete. ;-)

@bsipocz
Copy link
Member Author

bsipocz commented Jan 8, 2016

@hamogu - Sure, I'll make this work in the weekend. It's seems to be trivial to fix, I just didn't have any time today to iron it out (last night the builds were piling up and it was hopeless to wait for them to finish).

@crawfordsm
Copy link
Member

Thanks for submitting it @bsipocz and let me know if you need anything from us.

Thanks @hamogu for reviewing it. Once the tests are back up and running we will restart the outstanding pull requests.

@mwcraig
Copy link
Member

mwcraig commented Jan 9, 2016

@bsipocz -- I'll try to fix the Windows errors (may need to rewrite tests or mark some as skip on Windows ) if you want to take care of the travis fail...if you want to do Windows too, let me know, I won't stand in the way :)

@bsipocz
Copy link
Member Author

bsipocz commented Jan 9, 2016

Travis is basically fixed, the astropy version number is case sensitive, but I already have the PR in ci-helpers to make it insensitive.

The windows failures are a different case, they look genuine and I don't have much windows experience since win98, so can only try and hack around with the help of our mutual friend, google.
What about xfailing them in this PR and opening an issue for it, so you or whoever gets there first can fix it when have extra time?

@mwcraig
Copy link
Member

mwcraig commented Jan 9, 2016

xfailing sounds good. I've looked into the underlying issue before, it has to do with (at least in some of the cases) with overwriting files in Windows. Though the fail happens in io.fits, it is something in ImageFileCollection, I think, that is keeping a reference to a file that makes Windows unhappy.

@mwcraig
Copy link
Member

mwcraig commented Jan 9, 2016

Do you want me to do the xfails?

@bsipocz
Copy link
Member Author

bsipocz commented Jan 9, 2016

Nope, I already started it. Just waiting for the ci-helpers fix before pushing the commit.

@bsipocz
Copy link
Member Author

bsipocz commented Jan 10, 2016

@hamogu - the tests in your PRs will be also marked xfail on Appveyor until #276 is fixed, as they are within the marked class. They would most probably fail anyway as @mwcraig said above the problem lies somewhere in ImageFileCollection.

@bsipocz
Copy link
Member Author

bsipocz commented Jan 10, 2016

@mwcraig - This doesn't need a changelog entry, right? Or should the windows testing be mentioned somewhere?

@mwcraig
Copy link
Member

mwcraig commented Jan 10, 2016

@bsipocz -- thanks for all of your work on this!

@hamogu -- can you please rebase #274 and #275 after I merge?

@mwcraig
Copy link
Member

mwcraig commented Jan 10, 2016

@bsipocz -- nope, no need for changelog.

mwcraig added a commit that referenced this pull request Jan 10, 2016
Adding appveyor testing and opting in ci-helpers
@mwcraig mwcraig merged commit 2297902 into astropy:master Jan 10, 2016
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.

Add testing on appveyor
4 participants