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 tests in AppVeyor #3100

Closed
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@MarcelRaad
Member

MarcelRaad commented Oct 4, 2018

  • use in-tree build
  • remove redundant builds to compensate for the longer run times
  • use the preinstalled MSYS2 shell to run the tests

Test 1139 was disabled as the CMake build doesn't generate curl.1.
Test 500 was disabled as it failed with starttransfer vs total: 0.000001 0.000000 very often.

MarcelRaad added some commits Oct 3, 2018

AppVeyor: use in-tree build
Required to run the tests.
AppVeyor: run test suite
Use the preinstalled MSYS2 bash for that.
AppVeyor: Remove non-SSL non-test builds
They don't add much value.
AppVeyor: set custom install prefix
CMake's default has spaces and in 32-bit mode parentheses, which result
in syntax errors in curl-config.
AppVeyor: disable test 500
It almost always results in
"starttransfer vs total: 0.000001 0.000000".
I cannot reproduce this locally, so disable it for now.
@bagder

This comment has been minimized.

Member

bagder commented Oct 5, 2018

Lovely! Maybe switch off the verbose before landing but I love it!

@bagder

bagder approved these changes Oct 5, 2018

@jay

This comment has been minimized.

Member

jay commented Oct 5, 2018

I'm skeptical of disabling tests, we may have a bug somewhere causing that issue.

@bagder

This comment has been minimized.

Member

bagder commented Oct 5, 2018

I'm skeptical of disabling tests, we may have a bug somewhere causing that issue.

Sure, but I think improving the CI to run all tests except those two is such a huge improvement that I'm willing to risk that. Before this step, we didn't run any CI tests on windows.

Improving test 500 to work also on appveyor could be done as a high-prio separate issue since its not really an appveyor issue, its just that it outputs timing data and that seems risky to do in general and even more so on "cloud computers".

@MarcelRaad

This comment has been minimized.

Member

MarcelRaad commented Oct 5, 2018

I agree it would be better to not disable these tests. I'll try to enable one or both of them in the coming days after merging this.

MarcelRaad added a commit to MarcelRaad/curl that referenced this pull request Oct 5, 2018

AppVeyor: run test suite
Use the preinstalled MSYS2 bash for that.
Disable test 1139 as the CMake build doesn't generate curl.1.

Ref: curl#3070 (comment)
Closes curl#3100

MarcelRaad added a commit to MarcelRaad/curl that referenced this pull request Oct 5, 2018

AppVeyor: Remove non-SSL non-test builds
They don't add much value.

Closes curl#3100

MarcelRaad added a commit to MarcelRaad/curl that referenced this pull request Oct 5, 2018

AppVeyor: set custom install prefix
CMake's default has spaces and in 32-bit mode parentheses, which result
in syntax errors in curl-config.

Closes curl#3100

MarcelRaad added a commit to MarcelRaad/curl that referenced this pull request Oct 5, 2018

AppVeyor: disable test 500
It almost always results in
"starttransfer vs total: 0.000001 0.000000".
I cannot reproduce this locally, so disable it for now.

Closes curl#3100

@MarcelRaad MarcelRaad closed this in 1672661 Oct 5, 2018

jay added a commit to jay/curl that referenced this pull request Oct 5, 2018

appveyor: save build artifacts
- Save what is built by appveyor so it can be downloaded.

Being able to download what is built may help reproduce issues only
seen in appveyor that can't be reproduced locally. This way maybe we'll
be able to determine if it has something to do with the way it's being
built or the way it's being run.

Ref: curl#3100 (comment)

Closes #xxxx

@MarcelRaad MarcelRaad deleted the MarcelRaad:appveyor_tests branch Oct 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment