Skip to content

Improve python test coverage#5260

Draft
soswow wants to merge 6 commits into
AcademySoftwareFoundation:mainfrom
soswow:improve-python-test-coverage
Draft

Improve python test coverage#5260
soswow wants to merge 6 commits into
AcademySoftwareFoundation:mainfrom
soswow:improve-python-test-coverage

Conversation

@soswow

@soswow soswow commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

While working on #5254, where we're introducing changes that could surface bugs in pybind11 code that's in production today, I figured I'd try adding some tests.

This was a fairly loose-handed effort. To be upfront: most of it was LLM-generated. I did some research into which modules were lacking coverage the most, and those are what this PR targets. I haven't gone through every line of the generated code.

I hate being asked to review something that was clearly vibe-coded with no real effort put in, so I'm saying that here rather than pretending otherwise. I can try to review it properly, but without deep codebase knowledge it'll be slow and painful. Since these are just tests, my thinking was: even if there are silly bits here and there, it shouldn't hurt much, as long as things go green and we cover more logic and branches. Worth offering as-is.

soswow added 6 commits June 27, 2026 18:39
Signed-off-by: Aleksandr Motsjonov <soswow@gmail.com>
Signed-off-by: Aleksandr Motsjonov <soswow@gmail.com>
Signed-off-by: Aleksandr Motsjonov <soswow@gmail.com>
Signed-off-by: Aleksandr Motsjonov <soswow@gmail.com>
…lity

Signed-off-by: Aleksandr Motsjonov <soswow@gmail.com>
Signed-off-by: Aleksandr Motsjonov <soswow@gmail.com>
}
return py::object(py::cast(*spec));
ImageSpec spec = *spec_ptr;
return py::cast(spec);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

TextureSystem.imagespec() segfaulted because it returned Python objects while the GIL was released. Fixed by copying the ImageSpec after the C++ call, then casting with the GIL held.

@lgritz

lgritz commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator

It's only tests, can't break anything. But they do look reasonable to me.

You're not some kid doing a drive-by with AI-generated patches that they didn't look at and don't understand. You are systematically converting the python bindings from one toolkit to another and understand what you're doing, and along the way, you found some gaps in test coverage and filled them using tools that help automate the process. You and I both looked them over and they seem fine. They definitely add test coverage. Worst case is that they leave out something that should have been tested... which is also what the humans did here, or you wouldn't need to be adding this. I appreciate the caution here, but I think you're firmly in the area where these tools are extremely helpful.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants