Skip to content

cmake: drop CURL_DISABLE_TESTS option#16134

Closed
vszakats wants to merge 3 commits into
curl:masterfrom
vszakats:cm-dis-tests
Closed

cmake: drop CURL_DISABLE_TESTS option#16134
vszakats wants to merge 3 commits into
curl:masterfrom
vszakats:cm-dis-tests

Conversation

@vszakats
Copy link
Copy Markdown
Member

@vszakats vszakats commented Jan 30, 2025

curl builds tests with CMake when explicitly building the testdeps
target. It's not built by default. It seems overkill to have
a curl-specific variant of this (over CMake's BUILD_TESTING)
to disable generating this target.

Its history also doesn't make it obvious why this was necessary,
and there was a long debate how to do it, by the time the original
submitter abandoned CMake. The option also remained uninitialized
and thus undocumented.

Let me know if I missed something.

Ref: #6036
Ref: 3a1e798 #6072


I wonder if this option is useful. According to the linked Issues, its goal is to disable building tests for curl specifically, when in nested projects BUILD_TESTING would disable this for all other projects as well.

But, tests are not built in curl by default regardless of these settings. These control if the testdeps target is present or excluded at the "generation" step. Excluding this target saves near zero time there.

Test targets are build only when explicitly building the testdeps target. Maybe this target is also used in other projects? GitHub search reports this string being present in just a tiny number of projects: https://github.com/search?q=testdeps+language%3Acmake&type=code

If it would be a widely used name, perhaps renaming to curl-tests would be a better way to go. (and match how we fixed this for other target names.)

Or perhaps there is another shared mechanism to kick off (actually) building tests?

Any thoughts?

@github-actions github-actions Bot added the build label Jan 30, 2025
@vszakats vszakats marked this pull request as draft January 30, 2025 15:42
@vszakats vszakats changed the title cmake: document CURL_DISABLE_TESTS option cmake: document CURL_DISABLE_TESTS option (? or drop?) Jan 30, 2025
@vszakats vszakats changed the title cmake: document CURL_DISABLE_TESTS option (? or drop?) cmake: document CURL_DISABLE_TESTS option (or drop?) Jan 30, 2025
@vszakats vszakats changed the title cmake: document CURL_DISABLE_TESTS option (or drop?) cmake: document CURL_DISABLE_TESTS option (or drop/deprecate?) Jan 30, 2025
@vszakats vszakats changed the title cmake: document CURL_DISABLE_TESTS option (or drop/deprecate?) cmake: initialize and deprecate CURL_DISABLE_TESTS option Feb 7, 2025
@vszakats
Copy link
Copy Markdown
Member Author

vszakats commented Feb 7, 2025

@jzakrzewski @reddwarf69 ?

@vszakats vszakats changed the title cmake: initialize and deprecate CURL_DISABLE_TESTS option cmake: initialize and deprecate CURL_DISABLE_TESTS option (or delete/document?) Feb 7, 2025
@vszakats vszakats marked this pull request as ready for review February 7, 2025 12:11
@vszakats vszakats changed the title cmake: initialize and deprecate CURL_DISABLE_TESTS option (or delete/document?) cmake: initialize and deprecate CURL_DISABLE_TESTS option Feb 7, 2025
@vszakats vszakats changed the title cmake: initialize and deprecate CURL_DISABLE_TESTS option cmake: delete CURL_DISABLE_TESTS option Feb 7, 2025
@vszakats vszakats added tests tidy-up feature-window A merge of this requires an open feature window and removed documentation tidy-up labels Feb 7, 2025
@vszakats vszakats changed the title cmake: delete CURL_DISABLE_TESTS option cmake: drop CURL_DISABLE_TESTS option Feb 7, 2025
@vszakats vszakats changed the title cmake: drop CURL_DISABLE_TESTS option cmake: drop redundant CURL_DISABLE_TESTS option Feb 21, 2025
@vszakats vszakats changed the title cmake: drop redundant CURL_DISABLE_TESTS option cmake: drop CURL_DISABLE_TESTS option Feb 21, 2025
@vszakats vszakats closed this in bf82339 Feb 21, 2025
@vszakats vszakats deleted the cm-dis-tests branch February 21, 2025 11:06
pps83 pushed a commit to pps83/curl that referenced this pull request Apr 26, 2025
curl builds tests with CMake when explicitly building the `testdeps`
target. It's not built by default. It seems overkill to have
a curl-specific variant of this (over CMake's `BUILD_TESTING`)
to disable generating this target.

Its history also doesn't make it obvious why this was necessary,
and there was a long debate how to do it, by the time the original
submitter abandoned CMake. The option also remained uninitialized
and thus undocumented.

Let me know if I missed something.

Ref: curl#6036
Ref: 3a1e798 curl#6072
Closes curl#16134
vszakats added a commit to vszakats/curl that referenced this pull request Dec 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build cmake feature-window A merge of this requires an open feature window tests

Development

Successfully merging this pull request may close these issues.

1 participant