[libcamera-devel] tests: Add unset IPA environment variable test

1420 diff mbox series

There once was a crash  that was caused by an unset
LIBCAMERA_IPA_MODULE_PATH environment variable. (Fixed as of
"libcamera: ipa_manager: Fix handling of unset
LIBCAMERA_IPA_MODULE_PATH")

All tests have this environment variable set, which is why this error
wasn't caught for a while. Add a test (that is essentially the same as
list-cameras) that unsets this environment variable.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 test/ipa/ipa_env_var_test.cpp | 56 +++++++++++++++++++++++++++++++++++
 test/ipa/meson.build          |  1 +
 2 files changed, 57 insertions(+)
 create mode 100644 test/ipa/ipa_env_var_test.cpp

Hi Paul,

Thank you for the patch.

On Wed, Jun 12, 2019 at 07:15:05PM -0400, Paul Elder wrote:
> There once was a crash  that was caused by an unset

s/  / /

> LIBCAMERA_IPA_MODULE_PATH environment variable. (Fixed as of
> "libcamera: ipa_manager: Fix handling of unset
> LIBCAMERA_IPA_MODULE_PATH")
> 
> All tests have this environment variable set, which is why this error
> wasn't caught for a while. Add a test (that is essentially the same as
> list-cameras) that unsets this environment variable.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

How about instead unsetting it for a group of existing tests that don't
need IPAs ? That would save a bit of runtime by avoiding an extra test.

Another option would be to only set the variable for the test cases that
deal with IPAs. It may be more logical than manually unsetting it for a
test case that has nothing to do with IPAs.

> ---
>  test/ipa/ipa_env_var_test.cpp | 56 +++++++++++++++++++++++++++++++++++
>  test/ipa/meson.build          |  1 +
>  2 files changed, 57 insertions(+)
>  create mode 100644 test/ipa/ipa_env_var_test.cpp
> 
> diff --git a/test/ipa/ipa_env_var_test.cpp b/test/ipa/ipa_env_var_test.cpp
> new file mode 100644
> index 0000000..a221cd9
> --- /dev/null
> +++ b/test/ipa/ipa_env_var_test.cpp
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * ipa_env_var_test.cpp - IPA environment variable test
> + */
> +
> +#include <iostream>
> +
> +#include <libcamera/camera.h>
> +#include <libcamera/camera_manager.h>
> +
> +#include "test.h"
> +
> +using namespace std;
> +using namespace libcamera;
> +
> +class IPAEnvVarTest : public Test
> +{
> +protected:
> +	int init()
> +	{
> +		if (unsetenv("LIBCAMERA_IPA_MODULE_PATH")) {
> +			cerr << "failed to unset LIBCAMERA_IPA_MODULE_PATH"
> +			     << endl;
> +			return TestSkip;
> +		}
> +
> +		cm = CameraManager::instance();
> +		cm->start();
> +
> +		return TestPass;
> +	}
> +
> +	int run()
> +	{
> +		unsigned int count = 0;
> +
> +		for (const std::shared_ptr<Camera> &camera : cm->cameras()) {
> +			cout << "- " << camera->name() << endl;
> +			count++;
> +		}
> +
> +		return count ? 0 : -ENODEV;
> +	}
> +
> +	void cleanup()
> +	{
> +		cm->stop();
> +	}
> +
> +private:
> +	CameraManager *cm;
> +};
> +
> +TEST_REGISTER(IPAEnvVarTest)
> diff --git a/test/ipa/meson.build b/test/ipa/meson.build
> index bca39fa..7bc86fb 100644
> --- a/test/ipa/meson.build
> +++ b/test/ipa/meson.build
> @@ -1,5 +1,6 @@
>  ipa_test = [
>      ['ipa_test', 'ipa_test.cpp'],
> +    ['ipa_env_var_test', 'ipa_env_var_test.cpp'],
>  ]
>  
>  foreach t : ipa_test