Skip to content

Add matrix unit tests#156

Open
ekam313 wants to merge 1 commit into
thoth-tech:mainfrom
ekam313:feature/unit-testing-matrix
Open

Add matrix unit tests#156
ekam313 wants to merge 1 commit into
thoth-tech:mainfrom
ekam313:feature/unit-testing-matrix

Conversation

@ekam313
Copy link
Copy Markdown

@ekam313 ekam313 commented Mar 19, 2026

Description

Added Catch2 unit tests for matrix-related functions by converting the old manual matrix checks into automated assertions in coresdk/src/test/unit_tests/unit_test_matrix.cpp.

The new tests cover:

  • identity_matrix
  • matrix_inverse
  • matrix_to_string
  • scale_matrix
  • rotation_matrix
  • translation_matrix
  • matrix_multiply
  • combined rotation + translation matrix inverse check

This improves automated test coverage for matrix functionality and replaces manual output checking with repeatable u\nit tests.

Fixes # (issue)

Type of change

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

How Has This Been Tested?

Tested on Windows using MSYS2 MinGW64.

Commands used:

cd projects/cmake
cmake --preset Windows
cmake --build build/

cd ../../bin
./skunit_tests.exe "*matrix*" --success
./skunit_tests.exe --order rand

Checklist

  • Tested with sktest
  • Tested with skunit_tests

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

Copy link
Copy Markdown

@kyriesk kyriesk left a comment

Choose a reason for hiding this comment

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

Peer Review

I tested the unit_test_matrix.cpp on my local machine using a WSL setup, and all tests passed without issues. The code follows SplashKit’s coding standards and behaves functional.

Copy link
Copy Markdown

@222448082Ashen 222448082Ashen left a comment

Choose a reason for hiding this comment

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

General Information

  • Type of Change: New feature (Unit Testing)

Code Quality

  • Repository: Correct. The PR is made to splashkit-core.
  • Readability: High. The tests are well-organized by functionality and use a clear, property-based verification approach (e.g., verifying that a matrix multiplied by its inverse yields the identity matrix).
  • Maintainability: High. The tests rely on standard SplashKit types and formatting functions, making them robust against internal implementation changes as long as the API and output format remain stable.

Functionality

  • Correctness: The tests comprehensively cover the matrix_2d module:
    • Identity Property: Verifies identity_matrix() and that multiplying by identity is a no-op.
    • Inverse Operations: Correctly tests that inversion works for scaling, rotation, translation, and combinations thereof.
    • Edge Cases: Verifies that scale_matrix(1) and translation_matrix(0, 0) correctly return identity matrices.
    • Formatting: Explicitly tests the matrix_to_string output, which is crucial for debugging tools that rely on this format.
  • Impact on Existing Functionality: No impact on library code.

Testing

  • Test Coverage: This PR fills a significant gap in the unit test suite, as matrix operations are foundational to many other SplashKit modules (sprites, geometry, etc.).
  • Test Results: Logic check: The string comparison for matrix_to_string aligns perfectly with the setw(8) and setprecision(3) implementation found in matrix_2d.cpp.

Documentation

  • Documentation: The test code is clear and self-documenting.

Pull Request Details

  • Checklist Completion: All relevant items reviewed.

@222448082Ashen
Copy link
Copy Markdown

Recommendations & Observations

  1. Floating Point Comparison: The PR uses matrix_to_string comparison for inverse operations:
    REQUIRE(matrix_to_string(matrix_multiply(matrix, inv_matrix)) == matrix_to_string(identity_matrix()));
    This is an effective way to handle small floating-point variances in Catch2, as matrix_to_string uses setprecision(3), which naturally masks negligible errors. It's a clever and readable alternative to manual epsilon checks for these specific tests.
  2. Header Usage: The test includes #include "physics.h". While matrix_2d is used in physics, it's primarily defined in the geometry/graphics layers. Including matrix_2d.h or geometry.h directly might be slightly more precise, but physics.h works if it pulls in the necessary types.

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.

3 participants