Add support for unit tests, and a few unit tests #10636
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2TallTyler
reviewed
Apr 12, 2023
| TEST_CASE("GreatestCommonDivisorTest - Negative") | ||
| { | ||
| CHECK(4 == GreatestCommonDivisor(4, -52)); | ||
| // CHECK(3 == GreatestCommonDivisor(-27, 6)); // error - returns -3 |
There was a problem hiding this comment.
Commented code, and an error elsewhere?
There was a problem hiding this comment.
|
Had a look to see if hierarchical tests could be done, it looks like it's supported by catch2 by way of SECTIONs, but ctest itself doesn't do anything with those and only shows the top-level TEST_CASE. |
|
Nice work! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation / Problem
Tests for new and existing code will make development & refactoring easier.
And will make it easier to detect regressions and unintended changes.
See #9038 and #7796. Triggered by #10570 which adds some tests as constexpr code.
Description
Split building of OpenTTD into a "library" and the executable, by extracting the
main()functions into separate files. And then building the library without themain()function and the openttd executable is themain()function with the library linked. The library is not an actual library, but some construct in CMake that prevents building the same objects twice, since the library is going to be linked with the unit tests and then run.Add catch2 as third-parties library. It does not exist as library in ubuntu 20.04, so otherwise it would require updating those. Furthermore there were issues in getting it to work with Windows and Mac OS X. By third-partying it, it will just be available and do its thing.
Add unit test from #9038/#7796 and convert constexpr tests of #10570 to a proper unit test, and integrate catch2 into the build. Add an extra executable (openttd_test) that links the new "library" and the unit tests. With the integration it automatically gets picked up with ctest, and be run in parallel with the regression tests.
Limitations
I'm not sure whether OS/2 still builds.
I'm not sure whether the release workflows still function.
It has been a long time whack-a-moling to get this functioning, so I'm not sure whether I did the right things.
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.