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 a test suite for the Image class #45737

Merged
merged 1 commit into from
Feb 6, 2021

Conversation

OverloadedOrama
Copy link
Contributor

Part of #43440. I created a test suite for the Image class. This is my first time ever writing unit tests (and my second PR to Godot), so feedback is appreciated! The tests cover:

  • Image creation (create(), create_from_data(), copy_from())
  • Saving images to .png and .exr files.
  • Loading images from files (load() and load_x_from_buffer() methods, based on the work of @Calinou)
  • Getters, such as get_data(), get_size(), get_width(), get_height(), get_format(), get_used_rect(), get_rect(), get_pixel() and get_pixelv().
  • Image resizing with crop(), resize(), shrink_x2() and resize_to_po2().
  • Modifying the image's pixels with set_pixel(), set_pixelv(), fill(), blend_rect(), blit_rect(), flip_x() and flip_y().
  • Others, such as is_empty(), is_invisible(), is_compressed(), has_mipmaps().

The test suit is incomplete, the methods missing include most notably compression and mipmaps-related methods, bumpmap_to_normalmap(), srgb_to_linear() and some others, because I am not entirely sure how they work as I haven't used them so far.

@akien-mga akien-mga added this to the 4.0 milestone Feb 5, 2021
@akien-mga akien-mga requested a review from a team February 5, 2021 20:18
tests/test_image.h Outdated Show resolved Hide resolved
tests/test_image.h Outdated Show resolved Hide resolved
Ref<Image> image = memnew(Image(8, 4, false, Image::FORMAT_LA8));
CHECK_MESSAGE(
image->get_width() == 8,
"get_width() should return the expected value.");
Copy link
Contributor

@Xrayez Xrayez Feb 5, 2021

Choose a reason for hiding this comment

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

These type of messages are redundant in my opinion ("X expected to be Y"). I'd just use CHECK() macro for those. My rule of thumb is: if I start to write rationale which contains the "expected" word, I just replace CHECK_MESSAGE to CHECK. 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All right, I changed some of the checks to just CHECK().

@akien-mga akien-mga merged commit fa2f769 into godotengine:master Feb 6, 2021
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants