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

CI/tests: enable test target on TravisCI for CMake builds #6074

Closed
wants to merge 1 commit into from

Conversation

@snikulov
Copy link
Member

@snikulov snikulov commented Oct 14, 2020

Added test-nonflaky target to CMake builds

Ref: #6052

@snikulov snikulov requested a review from mback2k Oct 14, 2020
@snikulov snikulov force-pushed the snikulov:enable_tests_travis branch 2 times, most recently from 19fe6e0 to 4067ed2 Oct 14, 2020
@mback2k
Copy link
Member

@mback2k mback2k commented Oct 16, 2020

@snikulov can you try also enabling the suppression generation @bagder mentioned here: #6078 (comment), thanks!

@snikulov
Copy link
Member Author

@snikulov snikulov commented Oct 16, 2020

@snikulov can you try also enabling the suppression generation @bagder mentioned here: #6078 (comment), thanks!

It's done already.
Unfortunately, it shows no sign that it was enabled in TravisCI logs. 😕

@bagder
Copy link
Member

@bagder bagder commented Oct 16, 2020

That's so weird and I don't understand why we get no suppression rules shown. I propose we try adding one manually instead, here's my attempt:

diff --git a/tests/valgrind.supp b/tests/valgrind.supp
index 10b07314a..4aaeac5b4 100644
--- a/tests/valgrind.supp
+++ b/tests/valgrind.supp
@@ -1,5 +1,13 @@
+{
+  zstd_decompression-1.3.3
+  Memcheck:cond
+  fun:ZSTD_decompressStream
+  obj:/usr/lib/x86_64-linux-gnu/libzstd.so.1.3.3
+  fun:zstd_unencode_write
+}
+
 {
    libidn-idna_to_ascii-error
    Memcheck:Addr4
    fun:idna_to_ascii_4z
    fun:idna_to_ascii_8z
@bagder
Copy link
Member

@bagder bagder commented Oct 16, 2020

I also tried a configure build on ubuntu 18.04 with libzstd 1.3.3 on a local VM of mine, but I couldn't reproduce the valgrind issues there. 😮

@mback2k
Copy link
Member

@mback2k mback2k commented Oct 19, 2020

I also tried a configure build on ubuntu 18.04 with libzstd 1.3.3 on a local VM of mine, but I couldn't reproduce the valgrind issues there. open_mouth

Maybe the Windows version of libzstd contains different (#ifdef'ed) code that is causing the issue?

@bagder
Copy link
Member

@bagder bagder commented Oct 19, 2020

@mback2k: these valgrind reports happen on the two travis ubuntu 18.04 cmake builds...

@mback2k
Copy link
Member

@mback2k mback2k commented Oct 19, 2020

Ah sorry, mixed up the various issues in my head.

@bagder
Copy link
Member

@bagder bagder commented Oct 19, 2020

@snikulov try rebasing this now on master and force-push. The valgrind problems should be suppressed now.

@snikulov snikulov force-pushed the snikulov:enable_tests_travis branch 2 times, most recently from 4067ed2 to 2253538 Oct 19, 2020
@bagder
Copy link
Member

@bagder bagder commented Oct 20, 2020

TESTFAIL: These test cases failed: 1014 1139 

Test 1139 fails because the cmake build doesn't create docs/curl.1 so it can't check that the man page is correct. This should be fixed by making sure the file is created in this build as well, but a work-around could be to just disable test 1139 - it is being tested in other builds anyway.

Test 1014 looks like a more genuine problem in the cmake build. curl doesn't properly get IDN enabled in the build even though it found libidn2 present. Fixed in #6108

bagder added a commit that referenced this pull request Oct 20, 2020
... so that curl-config gets correct and makes test 1014 happy!

Ref: #6074
bagder added a commit that referenced this pull request Oct 20, 2020
This allows the build to enable IDN properly and it makes test 1014
happier.

Ref: #6074
bagder added a commit that referenced this pull request Oct 20, 2020
... so that curl-config gets correct and makes test 1014 happy!

Ref: #6074
Closes #6108
bagder added a commit that referenced this pull request Oct 20, 2020
This allows the build to enable IDN properly and it makes test 1014
happier.

Ref: #6074
Closes #6108
@mback2k
Copy link
Member

@mback2k mback2k commented Oct 30, 2020

@snikulov Please rebase and force-push in order to re-run the tests after @bagder's fixes.

@snikulov snikulov force-pushed the snikulov:enable_tests_travis branch from 2253538 to ad6b81e Nov 2, 2020
@snikulov
Copy link
Member Author

@snikulov snikulov commented Nov 2, 2020

@snikulov Please rebase and force-push in order to re-run the tests after @bagder's fixes.

@mback2k done.

@snikulov snikulov force-pushed the snikulov:enable_tests_travis branch from b296c51 to 9aa1f74 Nov 2, 2020
Added test-nonflaky target to CMake builds
Disabled test 1139 because the cmake build doesn't create docs/curl.1
@snikulov snikulov force-pushed the snikulov:enable_tests_travis branch from 9aa1f74 to 5b9aa59 Nov 3, 2020
@snikulov
Copy link
Member Author

@snikulov snikulov commented Nov 3, 2020

@mback2k @bagder now travis CI tests are green. I've also disabled 1139 as noted above.

@bagder
Copy link
Member

@bagder bagder commented Nov 3, 2020

Excellent! ❤️

@bagder bagder closed this in 9f43b28 Nov 3, 2020
@bagder
Copy link
Member

@bagder bagder commented Nov 3, 2020

Thanks!

1 similar comment
@mback2k
Copy link
Member

@mback2k mback2k commented Nov 3, 2020

Thanks!

@snikulov snikulov deleted the snikulov:enable_tests_travis branch Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.