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
Get unit tests working on all platforms. #33
Comments
@borrrden If just setting the path for CMake works, that would be sufficient. I think I spent a total of 15min trying to get it working last month, and then never got back to it. I will try again. |
CMake doesn't really even need to be involved at all in such a trivial part of the build process. As long as the catch header is somewhere that is in your include paths then you will be able to build it. The complication, I guess, comes from the fact that you expect the user to supply it. Is this something you did on purpose or just something that you picked up somewhere and have no opinion about? The answer to this determines how to move forward. For example you could, as we do, include the catch header right in your repo so that the only setup needed is to make sure that folder is in your include paths in the CMake project. |
Oh, I’m a total CMake novice. I’m sure that came out of cookbook somewhere. |
I don't think I'd want to grab the Catch2 headers and throw them into this repository. That doesn't seem right. I have a couple of Rust libraries that wrap C libraries. With those, I've taken to using a Git submodule to pull in the dependent project. That works really well with the Rust build system. I was thinking of trying that with a C++ project. |
OK. I got it. I saw a recommendation somewhere to just do a CMake build/install with Catch2, and that would work in a similar manner on Windows and Linux. When I tried it on Windows it failed, and I instantly gave up. This time I looked at the error and it was just an issue with file system privileges. So I tried the same thing in a console run as Administrator, and it worked, It installed into "C:\Program Files (x86)\Catch2" on my Win7 machine. It was just this in a Git Bash window run as Administrator on Win7:
After that, the sockkpp build worked with -DSOCKPP_BUILD_TESTS=ON. Now I'm getting those build errors with the tests. I'll start fixing them. |
Interesting. That was my initial assumption. Surprised that Linux did, but it seemed to make sense, since it requires the family when creating the socket.
This really stinks for portability.
Ah, yes. The call was properly failing on Windows, but it wasn't setting the last error. Now it sets it to
Interesting... Winsock implements the So now |
Thanks, @borrrden ! |
Installing the catch headers to the program files directory is quite awkward, and it makes it so that you cannot test this library on a machine that has no administrator access by default. But since Windows has no concept of a standard header directory (Leaving aside the vortex that is The remedy for someone with no admin access would be that they needed to patch the CMake file to take a hint directory for the Regarding portability......you'll get a resounding agreement from me. I don't know why they felt the need to create separate error codes when they could have easily reused the existing ones. Regardless, the one saving grace is that the errors are all the same as the POSIX errors, just prefixed with
I was so proud of myself too :p until I realized that the error codes are different most of the time for the cases in your tests....but maybe the above can be useful elsewhere in case they are the "same". |
Yeah, I was on the fence as whether to add specific error code checks to the unit tests or not. Many of them are weird corner cases that probably wouldn't be checked for specific errors in an actual application. As I add new tests, I will likely just check that the last error is something (non-zero), except for the places where it should check for specific errors like timeouts and non-blocking returns. On the other hand, it's cool to use the unit tests to show specific and non-portable behavior. So it's a toss-up. |
I was able to make it compile and run. The hardest part, actually, was fighting with CMake since Catch2 is not something I normally "install" since it's just one (or two) headers. Some CMake patching took care of that though, and the only thing left is the following issues (on Windows) in the unit tests causing 7 test and 8 assertion failures:
AF_INET
)sock.last_error() == EAFNOSUPPORT
needs to besock.last_error() == WSAEAFNOSUPPORT
). Also, often these codes areWSAEINVAL
orWSAENOTSOCK
instead.socket::pair
not implemented for Windows, so its corresponding test failsto_timeval
is not implemented on Windows, causing compilation failuresAll of the above are pretty easily fixable though.
EDIT OH I forgot,
initialize()
is never called during the tests so a lot of tests initially failed with the "not initialized" error. I fixed that by not auto generating the catch main function (GOTCHA: Include socket platform.h before catch.hpp to avoid odd errors from including headers in the wrong order)Originally posted by @borrrden in #25 (comment)
The text was updated successfully, but these errors were encountered: