Skip to content
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

Add test framework and some basic unit tests #9038

Closed
wants to merge 2 commits into from

Conversation

WimLeflere
Copy link
Contributor

Motivation

Tests for new and existing code will make development & refactoring easier.
And will make it easier to detect regressions and unintended changes.

Description

Added Catch2 header only test framework.
Math tests from #7796

Limitations

Prototype implementation to get feedback

Catch2 doesn't seem to be available in the Ubuntu 20.04 package repository
CMake FetchContent could be used instead: https://github.com/catchorg/Catch2/blob/devel/docs/cmake-integration.md

My CMake knowledge is to limited and/or the OpenTTD CMake setup is to advanced for me to do a correct test target integration. The current solution with CMake arguments is not ideal.
Other options, in 'quality' order

  1. create a openttd_lib which is used by openttd and openttd_test
    has the advantage of a single compile for most of the sources
  2. re-use the sources list for openttd in openttd_test
    two compiles, but no manual action needed to keep in sync
  3. specify the required source files for openttd_test separately
    lots of dependencies make it hard to include all the required files, manual updates needed for new files

added catch2 header only test framework
math tests from OpenTTD#7796
src/test/test_packet.cpp Outdated Show resolved Hide resolved
extend packet test, use byte values instead of characters
@2TallTyler
Copy link
Member

@WimLeflere Are you still interested in working on this? If not, I'd like to close it since it's a draft and not ready for review.

@WimLeflere
Copy link
Contributor Author

If there is no interest in adding a unit tests, the PR can be closed.

@2TallTyler
Copy link
Member

@LordAro I think you've previously been involved with unit tests. Thoughts?

@nikolas
Copy link
Member

nikolas commented Feb 20, 2023

I'd like to help with this - I have lots of experience with unit testing in Python and JavaScript. There are many areas where unit tests can help with development - e.g. I just introduced an unintended regression in OpenTTD's line drawing algorithm here: #10491

@nikolas
Copy link
Member

nikolas commented Feb 22, 2023

It looks like the blocker here is just a better CMake integration, possibly based on these docs:

I'm just learning CMake so it will take me some time to get up to speed.

@rubidium42
Copy link
Contributor

Test framework based on this PR got added in #10636.

@rubidium42 rubidium42 closed this Apr 16, 2023
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.

None yet

5 participants