-
Notifications
You must be signed in to change notification settings - Fork 40
Add create_bitmap unit test #138
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
base: main
Are you sure you want to change the base?
Conversation
222448082Ashen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description
This update adds a full set of unit tests for bitmap loading, creation, sizing, bounding shapes, and cleanup. The goal is to improve test coverage for the bitmap system and ensure behaviour is consistent across platforms.
Type of change
- Documentation / examples only
- New tests
Testing
The new tests cover:
- Loading multiple bitmaps and checking validity, width, and height
- Handling missing bitmap files
- Freeing individual bitmaps and freeing all bitmaps
- Retrieving bounding rectangle, bounding circle and center point
- Creating a new bitmap, checking dimensions, checking transparency, and drawing on it
Run using the usual CMake flow:
POSIX (Linux/macOS/MSYS2)
cd projects/cmake
mkdir -p build && cd build
cmake ..
cmake --build . --parallel
ctest --output-on-failureWindows PowerShell
cmake -S . -B build
cmake --build build --config Debug --parallel
ctest --test-dir build --output-on-failureRequired checklist
- Unit tests added under
coresdk/src/test/ - All tests pass locally
- No generated files were modified
- No public API changes
- No new resources required
Cross-platform notes
Tests passed locally on the available platform. CI will confirm Linux/macOS behaviour.
Files changed / reviewer notes
Key file:
coresdk/src/test/bitmap_tests.cpp— new test suite
Tests depend on the following sample assets already in the test resources folder:
rocket_sprt.pngfrog.pngbackground.png
No translator, binding or API changes involved.
Broccoli811
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a clear and well structured unit test that matches the PR description accurately. The test covers all essential behaviours requested in the task card targeting the "create_bitmap" function. You have included other tests like dimensions, transparency, and drawing which are nice additions.
I have run a test in my local machine, and the expected results matched the wanted behaviour.
The structure aligns with existing SplashKit testing patterns, and the use of SECTIONs keeps each behaviour isolated and easy to read. Everything in the description is reflected correctly in the code.
Right now, free_bitmap(bmp) only runs inside the final SECTION. If an earlier test fails such as dimension or transparency checks, the bitmap will not be freed. This isn’t significantly important but can cause memory to accumulate over multiple failed runs.
(You can take note of the previous test above where they free the bitmap outside of the SECTIONs)
Overall, this is a solid addition to the unit test suite and improves coverage of bitmap creation. Nice work!
e901d52 to
83cf2e6
Compare
|
Thanks @Broccoli811, good pick up! I've freed the bitmap outside of the SECTION as suggested. |
Description
Adds unit test for the
create_bitmapfunction from images.cpp (documentation here). The new test is named "can create and free a new bitmap".This test includes sections covering the following:
Note: Earlier unit tests performed on
load_bitmapused the terms "created"/"creating". Since this function creates a new empty bitmap rather than loading from a file, the names/comments for the earlier tests have been renamed for clarity.Type of change
How Has This Been Tested?
Built the test project as per unit testing instructions, running skunit_tests with all tests passed.
Testing Checklist
Checklist