dunst icon indicating copy to clipboard operation
dunst copied to clipboard

Moving tests to GLib?

Open bebehei opened this issue 7 years ago • 10 comments

We have recently introduced nesting suites/tests in https://github.com/dunst-project/dunst/pull/428#discussion_r156734027. This is an undocumented feature of greatest.h. But all in all, greatest has no official support.

In combination with #368, I looked at the glib logging capabilities and found out, that there is a whole testing framework included in GLib, which also has a testing method g_test_expect_message, which can actually require messages to get logged during tests.

GLib also offers to add subsuites for existing suites, which might get handy in our case.

bebehei avatar Jan 06 '18 18:01 bebehei

I've pushed this now forward to have a few samples to test it with GLib. Enough to judge, but not too much to have real work lost.

In combination with the logging stuff, commit e8e4af9 is not needed anymore in this testing branch 😉

What I can say about it right now:

  • nested test suites work (only) somewhat
  • They suppose to have a feature to link bugs with test cases.. I couldn't find out, how it's used in practice, but it looks somewhat cool.
  • it requires much more overhead in testing functions. I've cut this down with proper macros
  • the thinking isn't limited to a single test executable
  • catching expected messages in log work like a charm
  • The output on the commandline/stdout is not very pleasant. glib's reporting focus is at XML reports, which then get converted to HTML to read. This makes absolutely sense to for big projects. (but maybe not for dunst).

bebehei avatar Jan 07 '18 05:01 bebehei

I took a stab at this, it's not as straight-forward as it looks. in the option_parser and notification suite we depend on loading different configurations (read: global variables). It's easy to do in greatest, load it when you start the suite and free it after, but with glib we can only queue the tests and they are run at the end (maybe randomly) and as a result we have to add the init/deinit code in the tests themselves which ends with this mess. Any clues on how to make this better?

(note: I'm aware it fails, this is meant to prove a point haven't fully debugged it.)

tsipinakis avatar Sep 06 '18 10:09 tsipinakis

Sorry, but I fail to see your point. A few thoughts:

  • Why is it actually necessary to init/deinit for every test? Yeah, the order is undefined (which is IMHO good). But shouldn't the file reading be part of the test suite initialisation method? The option_get* methods aren't designed to modify anything!?
  • AFAIK you can run the GLib tests in separate processes. We could do it this way, so we don't have to bother around with deinitialisation of global symbols.
  • I'd have no objection against macros.

bebehei avatar Sep 10 '18 18:09 bebehei

Why is it actually necessary to init/deinit for every test?

The option_parser and notification suites require different config files loaded. If we call load_config both in option_parser and notification suite setup functions, the one will simply overwrite the other without any tests being run in between because we're just queuing them up in the test_suite_* functions and they are only run at the end.

tsipinakis avatar Sep 10 '18 19:09 tsipinakis

Ok, so we have to init/deinit. That's ok. What about simple macros for test start and end?

#define TEST_START(name) static void test_##name(void) {
#define TEST_END(name) } // END test_##name

#define TEST_OP_START(name) TEST_START(name) init_test_ini();
#define TEST_OP_END(name) deinit_test_ini(); TEST_END(name)

#define STR_EQ(a, b) g_assert_cmpstr(a, ==, b)
TEST_OP_START(next_section)
        const char *section = NULL;
        STR_EQ("bool",   (section = next_section(section)));
        STR_EQ("string", (section = next_section(section)));
        STR_EQ("path",   (section = next_section(section)));
        STR_EQ("int",    (section = next_section(section)));
        STR_EQ("double", (section = next_section(section)));
TEST_OP_END(next_section)

bebehei avatar Sep 10 '18 19:09 bebehei

I fucked up #541 and I have to fix it. But I guess, writing tests for queues.c first might be the best choice.

But before writing tests, I want to settle down, on which tool to use. So, what's your conclusion on here?

bebehei avatar Sep 25 '18 11:09 bebehei

Unless you're planning on using glib-specific test calls, work with greatest and I'll migrate that as well when I come back to it.

tsipinakis avatar Sep 25 '18 12:09 tsipinakis

@bebehei What's broken exactly? I don't want to delay 1.4 for a lot longer given the amount of changes currently in master, so can it wait for 1.5 or not?

tsipinakis avatar Nov 06 '18 19:11 tsipinakis

When you use fullscreen rules and have a fullscreen window. In some cases, unrelated notifications get inserted into displayed queue, too. I don't know the full details yet. And I actually want to start and write tests for this. But first, I have to add general tests for queues.c. (See #551). I'll finish this at the end of the week.

Please wait for this with the 1.4 release.

bebehei avatar Nov 06 '18 19:11 bebehei

I've moved this Issue to the next minor milestone. We definitely don't do it in 1.4.

But I would ask: Do we really want to move our testing infrastructure to GLib? Because I wouldn't say so anymore.

The only real benefit we would get is the log message checking. With greatest.h we would have to write some additonal code. But in the meantime I'm not convinced, that testing log messages might be a suitable thing to test.

The main thing we worry about, are the nested testsuites, right? The concept in GLib is better there. But we haven't experienced any problems with greatest either. And basing on the my past experiences, such things get reasonably fast resolved in greatest.

bebehei avatar Jan 30 '19 08:01 bebehei

I think that we can just use greatest now

bynect avatar Apr 12 '24 14:04 bynect