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
CMake based AppVeyor builds are no longer running tests #6052
Comments
Lessons learned: maybe we can modify the CI to check for an expected minimum number of successful tests being run. |
|
... and make TESTFAIL stand out a little better by adding newlines before and after. Reported-by: Marc Hörsken Issue: #6052
#6053 seems to do its job for the appveyor builds... |
/cc @Lekensteyn |
When cmake is used on appveyor, Was that perhaps done by the default |
It seems |
... for the builds were testing is enabled. Fixes #6052
(even for builds that don't actually run the tests) Fixes #6052
That's what the |
Ah right. But that doesn't seem to work (anymore)... |
I've tried numerous things now but I can't seem to fix this. I need help! |
@Lekensteyn @MarcelRaad can you a take a look at this maybe? Also: would #6036 be an option to solve this? |
I'm not familiar enough with CMake to help here, sorry. |
Perhaps appveyor updated their cmake? https://www.appveyor.com/updates/ |
I'm not 100% sure but suspect the following removal was the root cause.
|
After digging changes and logs, looks like it was an intentional change to avoid |
@mback2k Could you please check if it's looking good for you? https://ci.appveyor.com/project/snikulov/curl-ckq2k/build/job/5hn7cqysanf4df0p and https://ci.appveyor.com/project/snikulov/curl-ckq2k |
@snikulov looks better, but the test flags seem to be missing and I don't get the point of your second commit. TFLAGS should work on Windows. For AppVeyor the tests should be build by CMake, but then run using the existing run bash.exe runtests.pl execution. |
@mback2k yes, TFLAGS should work, but using ${...} is Unix specific. For Windows native shell it should be %...% AFAIK. I'm opposed bash usage, but you can try to run CMake from bash. Probably it will work. |
And my assumption not working :( Adding %TFLAGS% to the command produce the error:
|
@bagder @mback2k I'm not familiar with Perl, so could you please explain where env variable TFLAGS comes from? |
Okay, it seems like You will at least need to find a way to pass the following via |
yeah... still digging :) Added |
Updated appveyor.yml to set env variable TFLAGS and run tests Changed placeholder name from ${TFLAGS} to just TFLAGS to avoid platform shell issues Closes curl#6052
Updated appveyor.yml to set env variable TFLAGS and run tests Closes curl#6052
Updated appveyor.yml to set env variable TFLAGS and run tests Closes curl#6052
Updated appveyor.yml to set env variable TFLAGS and run tests Closes curl#6052
Updated appveyor.yml to set env variable TFLAGS and run tests Removed curly braces (${TFLAGS} -> $TFLAGS) Closes curl#6052
@mback2k Could you please check? https://ci.appveyor.com/project/snikulov/curl-ckq2k |
@snikulov Looks much better, but I don't yet see the arguments |
@mback2k How can you see this? |
@snikulov ah, thank you very much. I missed that and already wondered if that target is CMake specific and how |
Updated appveyor.yml to set env variable TFLAGS and run tests Removed curly braces (${TFLAGS} -> $TFLAGS) Move testdeps build to build step (per review comments) Closes curl#6052
Updated appveyor.yml to set env variable TFLAGS and run tests Removed curly braces (${TFLAGS} -> $TFLAGS) Move testdeps build to build step (per review comments) Closes curl#6052
Yes, we should! |
Ok then. Will submit another PR soon. |
I did this
I looked at the AppVeyor builds and saw the quite astonishing short build times for those builds with tests (approx. 10 minutes).
Then I saw this on many builds:
Manually going back in time and bisecting the AppVeyor history shows that commit ad64169 seems to break the tests:
Unfortunately that commit points to the wrong PR and I cannot see if it was sufficiently tested before merging.
I expected the following
Tests to be running like they did before ad64169:
cc @bagder @MarcelRaad @Lekensteyn
The text was updated successfully, but these errors were encountered: