Skip to content

fix TestBindSimple and TestBindCgoPackage#391

Open
b-long wants to merge 1 commit intomasterfrom
fix/bind-testbindsimple-testbindcgopackage
Open

fix TestBindSimple and TestBindCgoPackage#391
b-long wants to merge 1 commit intomasterfrom
fix/bind-testbindsimple-testbindcgopackage

Conversation

@b-long
Copy link
Copy Markdown
Member

@b-long b-long commented May 3, 2026

Summary

Fixes and un-skips two tests that have been skipped since PR #384 .

Fixes: #385
Rrelates-to: #370

  • TestBindSimple: re-enabled after fixing GIL ordering in generated CGo wrappers
  • TestBindCgoPackage: re-enabled after fixing an off-by-one in the cgo example's malloc call

Changes

  • bind/gen_func.go: restore the GIL before calling go2py converters — Python C API calls inside go2py must run with the GIL held, not after it has been released for the Go call
  • bind/gen_map.go, bind/gen_slice.go, bind/gen_struct.go: use add_checked_string_function for functions whose return type uses C.CString, so pybindgen generates correct ownership / free logic
  • _examples/cgo/cgo.go: fix malloc(strlen(str))malloc(strlen(str) + 1) (missing null terminator byte)
  • main_test.go: remove t.Skip() from TestBindSimple and TestBindCgoPackage
  • .github/workflows/ci.yml: add Go 1.23.x to the test matrix

* fix GIL ordering in generated CGo wrappers: restore GIL before go2py converters, use add_checked_string_function for C.CString return paths, and fix off-by-one in cgo.go malloc call
* remove skip for TestBindSimple & TestBindCgoPackage
* update Go version matrix in CI matrix
@b-long b-long force-pushed the fix/bind-testbindsimple-testbindcgopackage branch from f1592e9 to f2a9ad1 Compare May 3, 2026 14:39
@b-long b-long marked this pull request as ready for review May 3, 2026 14:56
@b-long
Copy link
Copy Markdown
Member Author

b-long commented May 3, 2026

Added the "help wanted" label here, to signal that I'd appreciate review from anyone in the community 🙂

Please if you have any thoughts, questions, concerns - I'd greatly appreciate your feedback!

Also, I'm sorry for the spam, but I'm cc'ing recent contributors to invite feedback 🙏 cc/ @rcoreilly @kousu @coffeemakingtoaster @pswilde

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.

TestBindSimple and TestBindCgoPackage should not be skipped

1 participant