search

gtk-rs / gir

Tool to generate rust bindings and user API for glib-based libraries
https://gtk-rs.org/gir/book/
MIT License
225 stars 99 forks source link

Make generated test failures more useful #1498

Closed nicopap closed 11 months ago

nicopap commented 11 months ago

Currently, when a sys compilation fails, the only context given is the command used to compile the code.

This is less than helpful. This PR adds all the compilation commands' output to the error message.

It also prints the message. unwrap doesn't format properly error messages, so we use println! as well. This results in the output being duplicated.

I consider the duplicate output to be acceptable, it's already far better than the current output, the fact that code is generated makes testing and developing a lot harder and we shouldn't let perfect be enemy of the better.

Here is a sample of the difference this commit makes:

BEFORE

thread 'cross_validate_layout_with_c' panicked at 'called `Result::unwrap()` on an `Err` value: "compilation command \"cc\" \"-Wno-deprecated-declarations\" \"-std=c11\" \"-D__USE_MINGW_ANSI_STDIO\" \"-I/usr/include/wpe-webkit-1.1\" \"-I/usr/include/glib-2.0\" \"-I/usr/lib/glib-2.0/include\" \"-I/usr/include/sysprof-4\" \"-I/usr/include/libsoup-3.0\" \"-I/usr/include/libmount\" \"-I/usr/include/blkid\" \"-pthread\" \"-I/usr/include/wpe-1.0\" \"-DWPE_ENABLE_XKB=1\" \"tests/layout.c\" \"-o\" \"/tmp/abiXdITDL/layout\" failed, exit status: 1"

AFTER

compilation command "cc" "-Wno-deprecated-declarations" "-std=c11" "-D__USE_MINGW_ANSI_STDIO" "-I/usr/include/wpe-webkit-1.1" "-I/usr/include/glib-2.0" "-I/usr/lib/glib-2.0/include" "-I/usr/include/sysprof-4" "-I/usr/include/libsoup-3.0" "-I/usr/include/libmount" "-I/usr/include/blkid" "-pthread" "-I/usr/include/wpe-1.0" "-DWPE_ENABLE_XKB=1" "tests/layout.c" "-o" "/tmp/abiXdITDL/layout" failed, exit status: 1
stdout:
stderr: In file included from tests/layout.c:7:
tests/manual.h:3:10: fatal error: cog/cog.h: No such file or directory
    3 | #include <cog/cog.h>
      |          ^~~~~~~~~~~
compilation terminated.

thread 'cross_validate_layout_with_c' panicked at 'called `Result::unwrap()` on an `Err` value: "compilation command \"cc\" \"-Wno-deprecated-declarations\" \"-std=c11\" \"-D__USE_MINGW_ANSI_STDIO\" \"-I/usr/include/wpe-webkit-1.1\" \"-I/usr/include/glib-2.0\" \"-I/usr/lib/glib-2.0/include\" \"-I/usr/include/sysprof-4\" \"-I/usr/include/libsoup-3.0\" \"-I/usr/include/libmount\" \"-I/usr/include/blkid\" \"-pthread\" \"-I/usr/include/wpe-1.0\" \"-DWPE_ENABLE_XKB=1\" \"tests/layout.c\" \"-o\" \"/tmp/abiXdITDL/layout\" failed, exit status: 1\nstdout: \nstderr: In file included from tests/layout.c:7:\ntests/manual.h:3:10: fatal error: cog/cog.h: No such file or directory\n    3 | #include <cog/cog.h>\n      |          ^~~~~~~~~~~\ncompilation terminated.\n"', tests/abi.rs:198:31
nicopap commented 11 months ago

I did use gir a bit more and this is a regression in certain cases.

It seems stderr actually is printed and is visible in the terminal when running cargo test. With this change, we lose the nice color formatting of the error.

I'm not sure why in the first place I didn't see the compilation error output. I'm fairly certain it didn't show up. Maybe it depends on what calls the compile function? I don't know.

Since this is a regression I'm going to put the pull request in draft mode.

nicopap commented 11 months ago

I now know why the tests previously failed without showing any errors. I'm closing this in favor of a fix for the problem in question.