Skip to content

Conversation

@dijidiji
Copy link

@dijidiji dijidiji commented Apr 17, 2025

Description

This is part of improving unit testing workflow.

Two additional CMake scripts are from the Catch2 2.x repo, used to register Catch2 tests with CTest, which is recommended per the Catch2 tutorial. CMakeLists.txt has been updated to integrate this.

CMake presets have been added to make it easier to build the unit tests from inside VS Code. In the future, presets could be added for building the project with code coverage.

Still needs testing on Mac.

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Built test project on WSL and Windows (MSYS2/Mingw64)

  • Built test project from terminal
  • Built test project from inside VS Code using presets

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have requested a review from ... on the Pull Request

@dijidiji dijidiji marked this pull request as ready for review April 26, 2025 04:50
@dijidiji dijidiji mentioned this pull request May 7, 2025
7 tasks
Copy link

@JPF2209 JPF2209 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked at the code and I have no suggestions for it, I approve of it

General Information

  • Type of Change:
    • Bug fix
    • Documentation update

Code Quality

  • Repository: Is this Pull Request is made to the correct repository? (Thoth-Tech NOT SplashKit)
  • Readability: Is the code easy to read and follow? If not are there comments to help understand
    the code?
  • Maintainability: Can this code be easily maintained or extended in the future?

Functionality

  • Correctness: Does the code meet the requirements of the task?
  • Impact on Existing Functionality: Has the impact on existing functionality been considered and
    tested?

This has been tested by running projects/cmake, then cmake . before make. Once I did this, I then ran both sktests and skunit_tests and these all worked.

Testing

  • Test Coverage: Are unit tests provided for new or modified code?
  • Test Results: Have all tests passed?

Documentation

  • Documentation: Are both inline and applicable external documentation updated and clear?

Pull Request Details

  • PR Description: Is the problem being solved clearly described?
  • Checklist Completion: Have all relevant checklist items been reviewed and completed?

@omckeon omckeon changed the base branch from develop to t1-2025 June 16, 2025 08:23
@macite macite merged commit 1828a9d into thoth-tech:t1-2025 Jun 20, 2025
3 checks passed
@dijidiji dijidiji deleted the ctest-integration branch July 14, 2025 06:10
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.

4 participants