Skip to content

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

Merged
merged 1 commit into from
Sep 11, 2021
Merged

meson: add tests #1044

merged 1 commit into from
Sep 11, 2021

Conversation

Tachi107
Copy link
Contributor

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.

@Tachi107
Copy link
Contributor Author

Tachi107 commented Sep 10, 2021

Tests failed because ServerTest.SlowPostFail fails sporadically on macOS (also in the non-Meson version), but you can see that they all passed in my repo, here.

@yhirose
Copy link
Owner

yhirose commented Sep 10, 2021

@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
Copy link
Owner

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?

Copy link
Contributor Author

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.crtbuild/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 :/

Copy link
Owner

@yhirose yhirose Sep 11, 2021

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!

Copy link
Contributor Author

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.

Copy link
Owner

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.

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.
@yhirose yhirose merged commit e3e28c6 into yhirose:master Sep 11, 2021
@Tachi107 Tachi107 deleted the meson-test branch September 15, 2021 20:41
@yhirose yhirose mentioned this pull request Jan 21, 2023
ExclusiveOrange pushed a commit to ExclusiveOrange/cpp-httplib-exor that referenced this pull request May 2, 2023
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants