Create unit test for function bitmap_name

[sam-stajnko]

Description

This PR includes unit tests for the Splashkit function bitmap_name. Tests include making sure that the returned name matches the name of the bitmap and checking if bitmap_name returns an empty string when an invalid bitmap reference is passed as a parameter.

Type of change

• New unit test added, with multiple sections

How Has This Been Tested?

To test, build the project and run the skunit_tests executable. Steps are provided on the Splashkit Documentation website if you are uncertain.

The test is named "name can be retrieved from bitmap" and should pass. If the test fails, let me know and we can track down the problem together.

Testing Checklist

• 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

[dijidiji]

The tests work for me, even in isolation and when run in a random order. I think you've considered a lot of edge cases here, which is great.

I could see a case made for keeping the file loading in if you deem it to be important to the test in question.

[dijidiji]

You can also use Catch2's string matchers for assertions with std::strings.

For example, to use Equals:

using Catch::Matchers::Equals;

Suggested change

	REQUIRE(returned_name == silly_name);
	REQUIRE_THAT(returned_name, Equals(silly_name));

I'm not sure if there's a reason to prefer using one over the other, just thought it was worth noting. So don't consider this a required change - I'm half reminding myself here for things to note down in the guide!

[sam-stajnko]

Hi @dijidiji ,

I have replaced load_bitmap with create_bitmap so there shouldn't be any more issues. As for your suggestion to use REQUIRE_THAT instead of REQUIRE, I have left the code unchanged because I think REQUIRE looks a bit cleaner.

Note, I have rebuilt skunit_tests and ran my tests for bitmap_name and all tests pass, indicating that it is working correctly.

If you have anything else that you think needs changing, don't hesitate to ask.
REgards,

Sam Stajnko.

[dijidiji]

Looks good, tests pass on my end as well! Approved.

[Broccoli811]

The new TEST_CASE for the bitmap_name function looks great. It clearly covers relevant scenarios including valid bitmaps, nullptr, freed bitmaps, dupe names, empty strings, and names with special characters.

I have run a test in my local machine, and the expected results matched the wanted behavior.

For my suggestion, perhaps consider extending the naming logic (blank, blank0, blank1, blank2 etc.) so we can see if it scales predictably, you can also extend this to freed bitmaps where we free multiple at once to confirm if multiple maps are updated correctly. Other than that, I can't think of any other scenarios.

Overall, very good implementation! Great Job! :)