[systemd-devel] [PATCH, v3 1/3] tests: added tests for unit_file_get_{state, list}

[Lennart Poettering]

On Tue, 21.10.14 15:21, Ken Sedgwick (ksedgwic at bonsai.com) wrote:

> This test constructs different unit file states and checks the output
> of unit_file_get_state and unit_file_get_list for each.
> 
> This test characterizes the current output of the master branch in
> preparation for a patch which improves the performance of unit state
> detection in the face of thousands of units.

Looks pretty good already! Love it! 

> +        root_dir = strappenda(TEST_DIR, "/test-enabled-root");
> +
> +        confirm_unit_state("nonexistent.service",	-ENOENT);
> +        confirm_unit_state("invalid.service", 		-EBADMSG);
> +        confirm_unit_state("disabled.service", 		UNIT_FILE_DISABLED);
> +        confirm_unit_state("another.service", 		UNIT_FILE_ENABLED);
> +        confirm_unit_state("runtime.service", 		UNIT_FILE_ENABLED_RUNTIME);
> +        confirm_unit_state("masked.service", 		UNIT_FILE_MASKED);
> +        confirm_unit_state("maskedruntime.service",	UNIT_FILE_MASKED_RUNTIME);
> +        confirm_unit_state("static.service", 		UNIT_FILE_STATIC);
> +        confirm_unit_state("maskedstatic.service",	UNIT_FILE_MASKED);
> +        confirm_unit_state("maskedruntimestatic.service", UNIT_FILE_MASKED_RUNTIME);
> +        confirm_unit_state("templating at .service",	UNIT_FILE_ENABLED);
> +        confirm_unit_state("templating at two.service",	UNIT_FILE_ENABLED);
> +        confirm_unit_state("templating at three.service", UNIT_FILE_ENABLED);
> +        confirm_unit_state("unique.service",                  UNIT_FILE_ENABLED);

Nitpick: looks weirdly aligned. Please don't use tabs in systemd (see
CODING_STYLE for more info).

> +
> +        h = hashmap_new(&string_hash_ops);

Please check for OOM. 

Adding an assert_se(h) after this line should suffice.

> +        r = unit_file_get_list(UNIT_FILE_SYSTEM, root_dir, h);
> +        assert_se(r >= 0);
> +
> +        HASHMAP_FOREACH(p, h, i) {
> +                UnitFileState s;
> +
> +                s = unit_file_get_state(UNIT_FILE_SYSTEM, root_dir,
> +                                        basename(p->path));

Nitpick: we don't really break lines that eagerly (also see CODING STYLE)

> +
> +                /* unit_file_get_list and unit_file_get_state are
> +                 * a little different in some cases.  Handle these
> +                 * cases here ...
> +                 */
> +                switch ((int)s) {

Why cast to (int)? Sounds unnecessary?

> +                case UNIT_FILE_ENABLED_RUNTIME:
> +                        assert_se(p->state == UNIT_FILE_ENABLED);
> +                        break;
> +                case -EBADMSG:
> +                        assert_se(p->state == UNIT_FILE_INVALID);
> +                        break;
> +                default:
> +                        assert_se(p->state == s);
> +                        break;
> +                }
> +
> +                fprintf(stderr, "%s (%s)\n",
> +                        p->path,
> +                        unit_file_state_to_string(p->state));
> +        }
> +        unit_file_list_free(h);
> +}


Lennart

-- 
Lennart Poettering, Red Hat