Skip to content

Add ut to project and create tests for each of the algorithms. #7

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

Open
wants to merge 40 commits into
base: main
Choose a base branch
from

Conversation

Sebanisu
Copy link
Contributor

@Sebanisu Sebanisu commented Mar 7, 2021

Add https://github.com/boost-ext/ut to the project to test it, and lead to having a CI run on it like it does for J. This will require compiler upgrades on to at least (GCC-9+, Clang-9.0+, Apple Clang-11.0.0+, MSVC-2019+*, Clang-cl-9.0+) just for the tests to build. Though the algorithm.hpp should still include just fine, into older compilers, Until we add newer standard code to it.

  • add .gitignore (I guess we did have one already, I copied the one from the j project as it filters more things out)
  • add static_assert(sizeof...(Is) != 0U); to transform and find. Since it's an error to not pass at least 1 iterator to these functions. Turns out it's okay to only have 1 range if the op or pred is coded to handle one range.
  • set library to c++20 (The .hpp files should still work on c++17 till we add c++20 code) (required by ut)
  • add ut to cmake
  • add test executable
  • add tests for algorithm.hpp
    • add some variadic operations. Such as:
      • plus, multiply, to test transform,
      • equal_to, not_equal_to for testing find, found
      • make_signed, make_unsigned these take a op and values change them to all signed or all unsigned and then pass to the op and return the value.
    • make sure to have constexpr tests to make sure the code works at constexpr time.
      • transform overload that takes a r-value for output range so I can pass a std::array and it returns the transformed version.
      • iota, I know we are getting a views::iota but it's only in one compiler as of now.
  • add test executable to ctest with add_test
  • add compiler warnings to test executable.
  • import .clang-format, I copied the one from the j project
  • appveyor.yml - I added this as a CI option. Though you might wanna set up your own CI like you did with j. Appveyor has to update gcc, ninja and cmake every time it runs. :P And I just did linux we probably wanna have build scripts for windows and mac. I created this with the appveyor website and exported the script to a yml. https://ci.appveyor.com/project/Sebanisu/an-algorithm-library
    • Build on MSVC 2019 - is the fastest because appveyor keeps MSVC up to date so we don't need to download updates.
    • Build on Clang 12
    • Build on GCC 10.2

I added extra functions to algorithm_test.cpp as I didn't want to add new functions to algorithm.h without checking with you first. These extra functions might be used in tests but might not be needed in the aal library.

resolves: #3

@Sebanisu
Copy link
Contributor Author

Sebanisu commented Mar 8, 2021

I think this is a good start. :)

@Sebanisu Sebanisu marked this pull request as ready for review March 8, 2021 18:34
@github-actions github-actions bot requested a review from codereport March 8, 2021 18:57
@Sebanisu
Copy link
Contributor Author

Sebanisu commented Mar 8, 2021

ninja -C build test is working in my other project. And I based my cmake code on that and this one it isn't working. So something is off in my cmake.

fixed enable_testing() needs to be ran on the root CMakeLists.txt

@Sebanisu
Copy link
Contributor Author

Sebanisu commented Mar 10, 2021

I found https://cliutils.gitlab.io/modern-cmake/chapters/testing.html so I added a some if statements in adb223c

Without the if statements the cTest was still looking for the tests even if they weren't built. So j's tests would show failed because it would try to run An-Algorithm-Library's tests.

https://cmake.org/cmake/help/latest/variable/CMAKE_PROJECT_NAME.html
https://cmake.org/cmake/help/latest/variable/PROJECT_NAME.html

Sebanisu added 23 commits March 18, 2021 07:39
This should only effect projects with cpp files.
…l iota.

Though maybe iota::view would work better. it only works in gcc i think.
based on the jsource one.
If you wanna try appveyor. I tried this build script out. It worked and ran tests. I'm not an expert though, so some of these commands might not be required.
seems the warnings have to be added to a interface library to work correctly. I also had to block the shadow errors to compile ut.
Though I'm using the verbose output, and it needs to build.
I found https://cliutils.gitlab.io/modern-cmake/chapters/testing.html

We need to put if statements around the enable testing and the add subdirectory.
Sebanisu added 13 commits March 18, 2021 07:39
this is some trial and error. It seemed to work on my code except clang doesn't have <concepts> so it errored on that.
we could probably swap out ranges code in places. though I am using a concept from ranges to check for a range.
though I can't get clang to fail on my computer so maybe it will on the CI
Really though if you're on msvc you're probably opening the project with a gui. Though we have linux commandline so I added the windows one.

I found out on the CI if you use the same ninja commands as linux ninja will use clang for windows.
There might be a better way? I just put what I found seems to work.
wrapped up the code used for making tests into a function so it's easier to add new tests.
suppress unreferenced local function has been removed.

I had this issue where one test was using a function and another wasn't so visual studio wasn't compiling because I have all warnings are errors turned on. In gcc adding `[[maybe_unused]]` was enough to fix this kind of issue but that didn't help in msvc.
probably won't need this warning suppressed. But the code is there if we end up needing it.
@Sebanisu
Copy link
Contributor Author

Appveyor's clang support might be limited till I solve a build script issue. Sebanisu/ToolsLibrary#6

Some c++20 features require libc++ to work like std::span. You might need to disable building on clang if you use too many new features of c++20.

@Sebanisu
Copy link
Contributor Author

MSVC has a bug with c++20 templated lambdas opened a ticket on microsoft Sebanisu/ToolsLibrary#4 though it might not be an issue here.

@Sebanisu
Copy link
Contributor Author

I was able to get it to build on clang12 with libc++ Might not be the best build script but it's working :)

https://cmake.org/cmake/help/v3.18/module/ExternalProject.html because if you make local changes the default behavior is to stash them and pop the stash after pulling the update. So it'll cause an error if there is a merge conflict.
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.

Use cmake to import a test library. So we can add testing to the project.
2 participants