Skip to content

Conversation

@AntonisDevStuff
Copy link

Additional tests for all from_* class methods of Color to close #3491

@AntonisDevStuff AntonisDevStuff requested a review from a team as a code owner November 4, 2025 19:24
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 4, 2025

📝 Walkthrough

Walkthrough

This PR adds test coverage for Color instance methods that mirror classmethod conversions. Tests verify that calling from_* methods on Color instances returns expected color component tuples, validating existing functionality without introducing API changes.

Changes

Cohort / File(s) Change Summary
Color instance method tests
test/color_test.py
Added tests for instance method calls to from_cmy, from_hsva, from_hsla, from_i1i2i3, from_normalized, and from_hex, each verifying that instance method results match their classmethod counterparts.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Add more tests for color class methods' accurately reflects the main change of adding test coverage for color class methods.
Description check ✅ Passed The description appropriately states the purpose of the PR and references the linked issue #3491 that requests additional test coverage.
Linked Issues check ✅ Passed The changes add tests for all from_* class methods (from_cmy, from_hsva, from_hsla, from_i1i2i3, from_normalized, from_hex) to verify they work when called on instances and produce expected results, fulfilling issue #3491 requirements.
Out of Scope Changes check ✅ Passed All changes are test-only additions to validate existing functionality; no out-of-scope modifications to core logic or unrelated areas were introduced.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
test/color_test.py (1)

790-790: Tests validate instance methods correctly, but consider testing with varied initial colors.

The instance method tests work correctly. However, to better align with issue #3491's requirement that "the original value of the instance should not affect the resulting color", consider testing with different initial colors beyond (0, 0, 0, 0). The test_from_hex method demonstrates this pattern well by using varied initial colors like (63, 12, 83) and (52, 31, 8, 255).

Example for test_from_cmy:

 def test_from_cmy(self): cmy = pygame.Color.from_cmy(0.5, 0.5, 0.5) cmy_tuple = pygame.Color.from_cmy((0.5, 0.5, 0.5)) cmy_instance = pygame.Color(0, 0, 0, 0).from_cmy(0.5, 0.5, 0.5) + cmy_instance2 = pygame.Color(255, 128, 64, 200).from_cmy(0.5, 0.5, 0.5) expected_cmy = (127, 127, 127) self.assertEqual(expected_cmy, cmy) self.assertEqual(expected_cmy, cmy_tuple) self.assertEqual(expected_cmy, cmy_instance) + self.assertEqual(expected_cmy, cmy_instance2)

Also applies to: 796-796, 808-808, 814-814, 826-826, 832-832, 844-844, 850-850, 860-860, 866-866

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a966d54 and 8de7e98.

📒 Files selected for processing (1)
  • test/color_test.py (7 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-01T20:22:31.010Z
Learnt from: Starbuck5 Repo: pygame-community/pygame-ce PR: 3573 File: src_c/_pygame.h:115-126 Timestamp: 2025-09-01T20:22:31.010Z Learning: In pygame-ce PR #3573, the PG_SURF_BitsPerPixel function implementation uses hardcoded return values of 32 for YUY2/UYVY/YVYU FOURCC formats because it's copying the exact logic from SDL's internal SDL_GetMasksForPixelFormat function. This ensures perfect compatibility between SDL2 and SDL3 behaviors rather than using the technically more accurate values that might be returned by SDL_GetPixelFormatDetails. 

Applied to files:

  • test/color_test.py
🧬 Code graph analysis (1)
test/color_test.py (1)
buildconfig/stubs/pygame/color.pyi (12)
  • Color (12-94)
  • from_cmy (54-54)
  • from_cmy (57-57)
  • from_hsva (60-60)
  • from_hsva (63-63)
  • from_hsla (66-66)
  • from_hsla (69-69)
  • from_i1i2i3 (72-72)
  • from_i1i2i3 (75-75)
  • from_normalized (78-78)
  • from_normalized (81-81)
  • from_hex (83-83)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: x86_64
  • GitHub Check: Pyodide build
  • GitHub Check: build (macos-14)
  • GitHub Check: build (windows-latest)
  • GitHub Check: Debian (Bookworm - 12) [s390x]
  • GitHub Check: dev-check
  • GitHub Check: x86
  • GitHub Check: debug_coverage (ubuntu-24.04, 3.10.17)
  • GitHub Check: debug_coverage (ubuntu-24.04, 3.14.0)
  • GitHub Check: AMD64
  • GitHub Check: msys2 (ucrt64, ucrt-x86_64)
  • GitHub Check: Debian (Bookworm - 12) [armv6]
  • GitHub Check: Debian (Bookworm - 12) [armv7]
  • GitHub Check: Debian (Bookworm - 12) [ppc64le]
  • GitHub Check: msys2 (clang64, clang-x86_64)
  • GitHub Check: aarch64
  • GitHub Check: x86_64
  • GitHub Check: build (ubuntu-22.04)
  • GitHub Check: i686
  • GitHub Check: msys2 (mingw64, x86_64)
🔇 Additional comments (1)
test/color_test.py (1)

878-882: Excellent test coverage for from_hex instance method!

These tests effectively demonstrate that the original instance value doesn't affect the from_hex result by using varied initial colors. This pattern aligns well with the requirements from issue #3491.

Also applies to: 893-896

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

Labels

None yet

1 participant