jhass / crystal-gobject

gobject-introspection for Crystal
BSD 3-Clause "New" or "Revised" License
127 stars 13 forks source link

RFC - Basic structure for generator testing without depending on GTK. #76

Closed hugopl closed 4 years ago

hugopl commented 4 years ago

Hi,

This is a RFC/PR, just to know if I'm on the right way and whether should I continue with this or not.

This project is getting bigger and it has no tests at all besides a check if all samples compile, so the idea is to have a dummy C library where we can replicate some situations found on some libraries, binding it and write test code using this binding.

For this I had to add few changes on current generator to be able to generate the binding for a library not in a usual location.

jhass commented 4 years ago

Awesome! I'm always a bit too lazy for this kind of stuff :P

I think I like the general direction, so just some minor things:

I'd also be curious to learn what problem the whole nodeps thingy is solving :)

hugopl commented 4 years ago
  • I'd keep the CI check of compiling the samples, I think it will provide a good smoke test even with a comprehensive testsuite

Makes sense, and add another shell script to run make on libtest and run crystal spec with the right env. vars and arguments to be passed on crystal spec.

  • What do you think of setting the GI_TYPELIB_PATH environment variable somewhere instead of messing with the search path and thus having to expose it everywhere in the API? Likewise setting LD_LIBRARY_PATH should be enough to avoid messing with prepend_library_path.

I didn't know the existence of this :-), way better.

  • Not sure bin/ feels like quite the right path to keep the compiled library, keeping it somewhere below spec feels more appropriate to me somehow :)

Maybe a spec/build dir… to put all .o, .so, .gir and .typelib

I'd also be curious to learn what problem the whole nodeps thingy is solving :)

When generating the bindings under the spec directory, I couldn't run the normal require_gobject macro, since it searches for the build_namespace.cr using a lib path, this is why I wrote macro code ({{ run ../src/… }}) directly into spec_helper.cr to load gobject and glib dependencies, this may probably be solved by another env. var that I don't know about the existence, so I would appreciate an elegant solution for this, I saw other shards with the same problem (running "run" macros on tests vs running on code), the one I remember is slang

jhass commented 4 years ago

Maybe a spec/build dir… to put all .o, .so, .gir and .typelib

Sounds good :)

When generating the bindings under the spec directory, I couldn't run the normal require_gobject macro, since it searches for the build_namespace.cr using a lib path

Mmh, I'd play a bit with a lib to src symlink like the one in samples, or setting CRYSTAL_PATH. Computing the path to build_namespace.cr using __DIR__ inside the macro could also be viable, not sure how it behaves inside a macro. If none of that helps I'd explore redefining require_gobject for the testsuite. If that also is unfruitful then I guess we can live with your solution, it's an internal thing anyhow.

hugopl commented 4 years ago

Now it's ready for a proper review.

Despite of this symlink hack and the current test added being stupid, I think it's good now. :-)

hugopl commented 4 years ago

finally :green_heart: on CI.

jhass commented 4 years ago

Great work <3