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
