Add test for enum-based overload resolution

[timosachsenberg]

Summary

• Adds test coverage to verify that overloaded methods with different enum parameter types dispatch correctly
• Confirms that scoped enum handling already supports overload resolution via isinstance() type checks
• Addresses issue overload resolution for enums #233 by documenting that the core functionality is already implemented

Changes

• Add overloaded process(MyEnum) and process(MyEnum2) methods to enums.hpp
• Declare process methods in enums.pxd
• Add tests verifying:
  • Correct dispatch based on enum type
  • Rejection of wrong enum types from different namespaces

Test plan

• pytest tests/test_code_generator.py::test_enums -v passes
• pytest tests/test_code_generator.py::test_enum_class_forward_declaration -v passes

Fixes #233

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests

  • Added comprehensive tests for enum overload resolution, validating correct dispatch for different enum types and type safety across namespaces.

• New Features

  • Enhanced enum processing with overload-based dispatch capabilities.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds test cases and implementation for enum overload resolution. Introduces overloaded process() methods in the Foo class accepting different enum types, returning strings, with corresponding Cython bindings to enable proper dispatch by enum type.

Changes

Cohort / File(s)	Summary
Test Suite tests/test_code_generator.py	Adds two new test cases (tests 5 and 6) validating enum overload resolution: verifies Foo.MyEnum and Foo.MyEnum2 dispatch to correct overloads returning b"MyEnum" or b"MyEnum2", and confirms Foo2.MyEnum.A from wrong namespace raises exception.
C++ Implementation tests/test_files/enums.hpp	Adds two overloaded process() member functions to class Foo accepting MyEnum and MyEnum2 respectively, both returning std::string. Includes <string> header for string type support.
Cython Declarations tests/test_files/enums.pxd	Adds import of C++ libcpp_string and declares two overloaded process() methods in Foo's extern C++ interface accepting Foo_MyEnum and Foo_MyEnum2, returning libcpp_string.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• [TEST] example for enums in namespaces #131: Both PRs modify enum test fixtures and namespace handling, introducing enum type distinctions and overload patterns across Foo and Foo2 classes.

Poem

🐰 Two enums where once was one,
Dispatch magic—overloads run!
Foo calls the right, by type divine,
MyEnum and MyEnum2 align,
Wrong namespace cast away with glee! 🎪

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add test for enum-based overload resolution' directly and accurately describes the main change: adding tests that verify enum-based overload resolution works correctly.
Linked Issues check	✅ Passed	The PR adds test coverage for enum-based overload resolution and confirms the functionality is already working, directly addressing issue #233's requirement for distinguishable enum types in Python.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to testing and supporting enum-based overload resolution: test additions, overloaded method declarations, and supporting C++ header/Cython interface changes.

✨ Finishing touches

• 📝 Generate docstrings

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

tests/test_code_generator.py (1)

121-123: Use a more specific exception type instead of bare Exception.

Catching bare Exception can mask unexpected errors. Based on Test 4 (line 110), autowrap appears to raise AssertionError for type validation failures. Consider using pytest.raises(AssertionError) or verify the actual exception type raised when no overload matches.

🔎 Verify the actual exception type raised

Run this test to determine the specific exception:

# Temporarily modify test to capture exception
try:
    foo.process(mod.Foo2.MyEnum.A)
    assert False, "Expected exception was not raised"
except Exception as e:
    print(f"Exception type: {type(e).__name__}, Message: {e}")
    raise

Then update the test to use the specific exception type:

-    with pytest.raises(Exception):
+    with pytest.raises(AssertionError):  # or TypeError, depending on actual behavior
         foo.process(mod.Foo2.MyEnum.A)

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0410f82 and 19584ea.

📒 Files selected for processing (3)

• tests/test_code_generator.py
• tests/test_files/enums.hpp
• tests/test_files/enums.pxd

🧰 Additional context used

🪛 Ruff (0.14.10)

tests/test_code_generator.py

122-122: Do not assert blind exception: Exception

(B017)

⏰ 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). (8)

• GitHub Check: test (==3.1.0, 3.13)
• GitHub Check: test (==3.1.0, 3.10)
• GitHub Check: test (==3.1.0, 3.11)
• GitHub Check: test (==3.2.0, 3.10)
• GitHub Check: test (==3.2.0, 3.11)
• GitHub Check: test (==3.2.0, 3.12)
• GitHub Check: test (==3.2.0, 3.13)
• GitHub Check: test (==3.1.0, 3.12)

🔇 Additional comments (3)

tests/test_code_generator.py (1)

113-119: LGTM! Overload resolution test correctly validates enum-based dispatch.

The test correctly verifies that the overloaded process() methods dispatch to the appropriate implementation based on the enum type passed, confirming the core functionality described in PR #234.

tests/test_files/enums.hpp (1)

18-18: LGTM! Clean implementation of enum-based overload resolution.

The addition of #include <string> and the two overloaded process() methods correctly demonstrate C++ overload resolution based on enum type. The return values match the test expectations.

Also applies to: 48-52

tests/test_files/enums.pxd (1)

2-2: LGTM! Cython declarations correctly map the C++ overloads.

The import of libcpp.string and the declarations of the two overloaded process() methods properly expose the C++ functionality to Python, enabling enum-based overload resolution.

Also applies to: 48-50

[timosachsenberg]

hmm seems to work right?

[jpfeuffer]

Yes looks correct