[pulseaudio-discuss] [PATCH] tests: Add tests for alsa-mixer paths

[Tanu Kaskinen]

On Fri, 2013-03-22 at 15:37 +0100, David Henningsson wrote:
> It checks all files in the mixer/paths directory and checks
>  - that the file can be parsed without errors
>  - that the file is actually shipped in the makefile
> 
> Signed-off-by: David Henningsson <david.henningsson at canonical.com>
> ---
> 
> After I realized that my previous headset-mic patch did not event ship the new file,
> I also realized I might do the same mistake again.
> 
> I guess this could be enhanced later with checking that profile-sets do not point
> to non-existent paths etc, but this will do for now.
> 
>  src/Makefile.am                  |    7 +++
>  src/tests/alsa-mixer-path-test.c |  110 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 117 insertions(+)
>  create mode 100644 src/tests/alsa-mixer-path-test.c
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 7685e0c..3326a2c 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -297,6 +297,8 @@ endif
>  if HAVE_ALSA
>  TESTS_norun += \
>  		alsa-time-test
> +TESTS_default += \
> +		alsa-mixer-path-test
>  endif
>  
>  if HAVE_TESTS
> @@ -545,6 +547,11 @@ alsa_time_test_LDADD = $(AM_LDADD) $(ASOUNDLIB_LIBS)
>  alsa_time_test_CFLAGS = $(AM_CFLAGS) $(ASOUNDLIB_CFLAGS)
>  alsa_time_test_LDFLAGS = $(AM_LDFLAGS) $(BINLDFLAGS)
>  
> +alsa_mixer_path_test_SOURCES = tests/alsa-mixer-path-test.c
> +alsa_mixer_path_test_CFLAGS = $(AM_CFLAGS) $(LIBCHECK_CFLAGS) $(ASOUNDLIB_CFLAGS)
> +alsa_mixer_path_test_LDADD = $(AM_LDADD) libpulsecore- at PA_MAJORMINOR@.la libpulse.la libpulsecommon- at PA_MAJORMINOR@.la libalsa-util.la
> +alsa_mixer_path_test_LDFLAGS = $(AM_LDFLAGS) $(BINLDFLAGS) $(LIBCHECK_LIBS)
> +
>  usergroup_test_SOURCES = tests/usergroup-test.c
>  usergroup_test_LDADD = $(AM_LDADD) libpulsecore- at PA_MAJORMINOR@.la libpulse.la libpulsecommon- at PA_MAJORMINOR@.la
>  usergroup_test_CFLAGS = $(AM_CFLAGS) $(LIBCHECK_CFLAGS)
> diff --git a/src/tests/alsa-mixer-path-test.c b/src/tests/alsa-mixer-path-test.c
> new file mode 100644
> index 0000000..2460c66
> --- /dev/null
> +++ b/src/tests/alsa-mixer-path-test.c
> @@ -0,0 +1,110 @@
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <check.h>
> +#include <dirent.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +
> +#include <pulse/pulseaudio.h>
> +#include <pulsecore/log.h>
> +#include <pulsecore/core-util.h>
> +#include <pulsecore/strlist.h>
> +#include <modules/alsa/alsa-mixer.h>
> +
> +
> +static const char *get_default_paths_dir(void) {
> +    if (pa_run_from_build_tree())
> +        return PA_BUILDDIR "/modules/alsa/mixer/paths/";

This doesn't work if the build directory is different from the source
directory. Instead of PA_BUILDDIR, this should use PA_SRCDIR, which
doesn't currently exist, but it should be possible to add it.

> +    else
> +        return PA_ALSA_PATHS_DIR;
> +}
> +
> +static pa_strlist *load_makefile() {
> +    FILE *f;
> +    bool lookforfiles = false;
> +    char buf[2048];
> +    pa_strlist *result = NULL;
> +    const char *Makefile = PA_BUILDDIR "/Makefile";
> +
> +    f = pa_fopen_cloexec(Makefile, "r");
> +    fail_unless(f != NULL); /* Consider skipping this test instead of failing if Makefile not found? */

If you want my opinion to the question, I think it's fine to assume that
Makefile is always found.

> +    while (!feof(f)) {
> +        if (!fgets(buf, sizeof(buf), f)) {
> +            fail_unless(feof(f));

The loop condition is !feof(f), so this will always fail. The if could
be replaced with fail_unless(fgets(buf, sizeof(buf), f));

> +            break;
> +        }
> +        if (strstr(buf, "dist_alsapaths_DATA = \\") != NULL) {
> +           lookforfiles = true;
> +           continue;
> +        }
> +        if (!lookforfiles)
> +           continue;
> +        if (!strstr(buf, "\\"))
> +           lookforfiles = false;
> +        else
> +           strstr(buf, "\\")[0] = '\0';
> +        pa_strip(buf);
> +        pa_log_debug("Shipping file '%s'", pa_path_get_filename(buf));
> +        result = pa_strlist_prepend(result, pa_path_get_filename(buf));
> +    }
> +    fclose(f);
> +    return result;
> +}
> +
> +
> +START_TEST (mixer_path_test) {
> +    DIR *dir;
> +    struct dirent *ent;
> +    pa_strlist *ship = load_makefile();

This strlist is never freed.

> +    const char *pathsdir = get_default_paths_dir();
> +    pa_log_debug("Analyzing directory: '%s'", pathsdir);
> +
> +    dir = opendir(pathsdir);
> +    fail_unless(dir != NULL);
> +    while ((ent = readdir(dir)) != NULL) {
> +        pa_alsa_path *path;
> +        if (strcmp(ent->d_name, ".") == 0 || strcmp(ent->d_name, "..") == 0)

pa_streq() could be used here.

> +            continue;
> +        pa_log_debug("Analyzing file: '%s'", ent->d_name);
> +
> +        /* Can the file be parsed? */
> +        path = pa_alsa_path_new(pathsdir, ent->d_name, PA_ALSA_DIRECTION_ANY);
> +        fail_unless(path != NULL);
> +
> +        /* Is the file shipped? */
> +        if (ship) {
> +            pa_strlist *n;
> +            bool found = false;
> +            for (n = ship; n; n = pa_strlist_next(n))
> +                found |= pa_streq(ent->d_name, pa_strlist_data(n));
> +            fail_unless(found);
> +        }
> +    }
> +    closedir(dir);
> +}
> +END_TEST
> +
> +int main(int argc, char *argv[]) {
> +    int failed = 0;
> +    Suite *s;
> +    TCase *tc;
> +    SRunner *sr;
> +
> +    if (!getenv("MAKE_CHECK"))
> +        pa_log_set_level(PA_LOG_DEBUG);
> +
> +    s = suite_create("Alsa-mixer-path");
> +    tc = tcase_create("alsa-mixer-path");
> +    tcase_add_test(tc, mixer_path_test);
> +    tcase_set_timeout(tc, 5 * 60);

5 minutes seems a bit excessive for this test...

-- 
Tanu