-
Notifications
You must be signed in to change notification settings - Fork 2.4k
meson: add tests #1044
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
meson: add tests #1044
Conversation
Tests failed because |
@Tachi107, thank you for the pull request. I don't know about meson build system though, I reviewed the code. Could you follow my comments? Thanks! |
@@ -0,0 +1,7 @@ | |||
# SPDX-FileCopyrightText: 2021 Andrea Pappacoda |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you remove this file from 'test/www/dir' directory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately I can't.
The "old" tests are run in-tree, and require ca-bundle.crt
, image.jpg
www/dir*/*
to be ran.
In-tree builds are disallowed in Meson (and discouraged in CMake), so it is necessary to copy the required files in the build directory to recreate the same directory structure as the one found in the source tree (that is, test/ca-bundle.crt
→ build/test/ca-bundle.crt
, etc). Copying files to the build dir is accomplished by the configure_file(input: 'filename', output: 'filename', copy: true)
function, where the input is a file name relative to the same directory as the current meson.build
file. Copying a file from a subdirectory is not allowed, as Meson guaranties that a file in a certain file in the build dir (like build/test/www/dir/index.html
) has to come from the same path of the source tree (test/www/dir/index.html
).
This makes debugging easier, but obviously has the downside of having to create a meson.build
file in the same dir as the files you want to copy. So I can't remove these three files :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'www', 'www2' and 'www3' are just htdocs folders, and I may add more such folders like 'www4' in the future. I would not like to remember adding 'meson.build' file each time I add a new data directory, and it puts an extra burden on me. I personally don't care for testing on Meson build system when testing, because I only use just plain 'Make' files and it's enough. (CMake in this project just focus on build and install, but not test.)
So I am thinking not to accept this pull request. Sorry for my decision, but thanks for your efforts so far and understanding my situation!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand your concern, but don't worry, the worst that could happen if you don't add the meson.build file is a failed test, better than no tests at all :)
I've committed to maintain the Meson part of this repo, so if something Meson-related has issues I'll fix it as soon as possible (the same way @sum01 is the "maintainer" of the CMake portion of cpp-httplib).
If for whatever reason I stop maintaining something that needs to be updated (like the tests) you're always free to remove it (like you did with CMake some time ago).
Having tests that ensure that Meson users do not use a broken build of the library (when not using the header-only version) are important to ensure that cpp-httplib will always work fine for everyone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glad to see your willingness! I'll merge it.
2ee8e6d
to
a1084fc
Compare
This integrates the "main" test suite (test/test.cc) in Meson. This allows to run the tests in the CI with the Meson-built version of the library to ensure that nothing breaks unexpectedly. It also simplifies life of downstream packagers, that do not have to write a custom build script to split the library and run tests but can instead just let Meson do that for them.
a1084fc
to
e7457be
Compare
This integrates the "main" test suite (test/test.cc) in Meson. This allows to run the tests in the CI with the Meson-built version of the library to ensure that nothing breaks unexpectedly. It also simplifies life of downstream packagers, that do not have to write a custom build script to split the library and run tests but can instead just let Meson do that for them.
This integrates the "main" test suite (
test/test.cc
) in Meson.This allows to run the tests in the CI with the Meson-built version of the library to ensure that nothing breaks unexpectedly.
It also simplifies life of downstream packagers, that do not have to write a custom build script to split the library and run tests but can instead just let Meson do that for them.