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

Closed
wants to merge 5 commits into from
Closed

Conversation

MarcelRaad
Copy link
Member

  • 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.

Required to run the tests.
Use the preinstalled MSYS2 bash for that.
They don't add much value.
CMake's default has spaces and in 32-bit mode parentheses, which result
in syntax errors in curl-config.
It almost always results in
"starttransfer vs total: 0.000001 0.000000".
I cannot reproduce this locally, so disable it for now.
@bagder
Copy link
Member

bagder commented Oct 5, 2018

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

@jay
Copy link
Member

jay commented Oct 5, 2018

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

@bagder
Copy link
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
Copy link
Member Author

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
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
They don't add much value.

Closes curl#3100
MarcelRaad added a commit to MarcelRaad/curl that referenced this pull request Oct 5, 2018
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
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
- 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 appveyor_tests branch October 5, 2018 20:13
@lock lock bot locked as resolved and limited conversation to collaborators Jan 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants