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

Run MSVC tests on AppVeyor #723

Merged
merged 1 commit into from
Jun 28, 2016
Merged

Run MSVC tests on AppVeyor #723

merged 1 commit into from
Jun 28, 2016

Conversation

KindDragon
Copy link
Contributor

@KindDragon
Copy link
Contributor Author

@BillyDonahue please review and merge (also create configuration at https://www.appveyor.com/) before somebody break Windows build 😄

@KindDragon
Copy link
Contributor Author

@BillyDonahue Maybe you can try to find additional maintainer inside Google? 😏

@trebinor
Copy link

Can anyone comment on when this will be merged to master? My project is eagerly awaiting a fix contained in these changes.

@BillyDonahue
Copy link
Contributor

I'm happy to see a Windows CI tool integration here.
I'm curious whether we can do the code changes separately though.
Thanks!

@KindDragon
Copy link
Contributor Author

KindDragon commented Jun 23, 2016

I can move changes form this Pull request, but build will failing for some VS versions

@BillyDonahue
Copy link
Contributor

That's fine temporarily. Then you can be the hero and send a fix
immediately afterward.

On Thu, Jun 23, 2016 at 6:23 AM, Arkady Shapkin notifications@github.com
wrote:

I can move changes form this Pull request, but build will failing for some
VS


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#723 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AINnRFa3RG-GPa2AljVWGNXBCEW-hMIwks5qOl60gaJpZM4HhB7w
.

@KindDragon
Copy link
Contributor Author

😄 ok

@KindDragon
Copy link
Contributor Author

It's ready for merge

@@ -12,4 +12,4 @@ cmake -Dgtest_build_samples=ON \
-DCMAKE_CXX_FLAGS=$CXX_FLAGS \
../../$GTEST_TARGET
make
make test
CTEST_OUTPUT_ON_FAILURE=1 make test
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain to me what this change does to the travis build?
Excuse my ignorance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Output anything outputted by the test program if the test should fail. 

That environment variable will show test output on Travis to simplify debugging failed tests

@BillyDonahue
Copy link
Contributor

I'm wondering what's up with the Travis build now?
Something about apt-get install failing. Can you help figure that out?
https://travis-ci.org/google/googletest/jobs/140203419#L406

@KindDragon KindDragon force-pushed the master branch 4 times, most recently from 4bfce41 to 23a8cda Compare June 26, 2016 20:43
@KindDragon
Copy link
Contributor Author

I can't test it, because latest master commit broke Travis build

@BillyDonahue
Copy link
Contributor

fixed now.

On Sun, Jun 26, 2016 at 4:47 PM, Arkady Shapkin notifications@github.com
wrote:

I can't test it, because latest master commit broke Travis build


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#723 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AINnRHV_1JlzmHeypBnow4DDftvYgpWxks5qPuVPgaJpZM4HhB7w
.

@KindDragon
Copy link
Contributor Author

Rebased on latest changes.

Moved VS2015 fixes to PR #811

@KindDragon
Copy link
Contributor Author

Is there any problems with this pull request ?

@BillyDonahue BillyDonahue merged commit 8134585 into google:master Jun 28, 2016
@KindDragon
Copy link
Contributor Author

So now you can register on https://ci.appveyor.com/projects site and add badge to readme.md

@BillyDonahue
Copy link
Contributor

thanks for doing this!

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.

3 participants