[libcamera-devel,1/5] test: Move test objects to libtest

Message ID 20181221081311.3291-2-kieran.bingham@ideasonboard.com
State Accepted
Headers show
Series
  • test: Define libtest
Related show

Commit Message

Kieran Bingham Dec. 21, 2018, 8:13 a.m. UTC
Create a subdirectory to contain the libtest helper library.

Define two variables to clarify when tests are aimed at public or
internal components.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 test/libtest/meson.build    |  7 +++++++
 test/{ => libtest}/test.cpp |  0
 test/{ => libtest}/test.h   |  0
 test/meson.build            | 20 ++++++++++++++------
 4 files changed, 21 insertions(+), 6 deletions(-)
 create mode 100644 test/libtest/meson.build
 rename test/{ => libtest}/test.cpp (100%)
 rename test/{ => libtest}/test.h (100%)

Comments

Jacopo Mondi Dec. 21, 2018, 3:09 p.m. UTC | #1
Hi Kieran,

On Fri, Dec 21, 2018 at 08:13:07AM +0000, Kieran Bingham wrote:
> Create a subdirectory to contain the libtest helper library.
>
> Define two variables to clarify when tests are aimed at public or
> internal components.
>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  test/libtest/meson.build    |  7 +++++++
>  test/{ => libtest}/test.cpp |  0
>  test/{ => libtest}/test.h   |  0
>  test/meson.build            | 20 ++++++++++++++------
>  4 files changed, 21 insertions(+), 6 deletions(-)
>  create mode 100644 test/libtest/meson.build
>  rename test/{ => libtest}/test.cpp (100%)
>  rename test/{ => libtest}/test.h (100%)
>
> diff --git a/test/libtest/meson.build b/test/libtest/meson.build
> new file mode 100644
> index 000000000000..b998154dd8d3
> --- /dev/null
> +++ b/test/libtest/meson.build
> @@ -0,0 +1,7 @@
> +libtest_sources = files([
> +    'test.cpp',
> +])
> +
> +libtest = static_library('libtest', libtest_sources)
> +
> +libtest_includes = include_directories('.')
> diff --git a/test/test.cpp b/test/libtest/test.cpp
> similarity index 100%
> rename from test/test.cpp
> rename to test/libtest/test.cpp
> diff --git a/test/test.h b/test/libtest/test.h
> similarity index 100%
> rename from test/test.h
> rename to test/libtest/test.h
> diff --git a/test/meson.build b/test/meson.build
> index da0aea9678d1..50ec11853203 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -1,11 +1,19 @@
> -libtest_sources = files([
> -    'test.cpp',
> -])
> +subdir('libtest')
>
> -libtest = static_library('libtest', libtest_sources)
> +test_libraries = [libcamera, libtest]
> +
> +test_includes_public = [
> +  libtest_includes,
> +  libcamera_includes,
> +]
> +
> +test_includes_internal = [

That's not really 'internal' if it includes the public includes :)

> +  test_includes_public,
> +  libcamera_internal_includes,
> +]
>
>  test_init = executable('test_init', 'init.cpp',
> -                       link_with : libcamera,
> -                       include_directories : libcamera_includes)
> +                       link_with : test_libraries,
> +                       include_directories : test_includes_public)

What about a test/tests/ directory? Or do we expect to add all tests
under test/ itself? Eg. test/libcamera/media_device_test.cpp ?

Thanks
  j
>
>  test('Initialisation test', test_init)
> --
> 2.17.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Kieran Bingham Dec. 21, 2018, 3:15 p.m. UTC | #2
Hi Jacopo,

On 21/12/2018 15:09, jacopo mondi wrote:
> Hi Kieran,
> 
> On Fri, Dec 21, 2018 at 08:13:07AM +0000, Kieran Bingham wrote:
>> Create a subdirectory to contain the libtest helper library.
>>
>> Define two variables to clarify when tests are aimed at public or
>> internal components.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>> ---
>>  test/libtest/meson.build    |  7 +++++++
>>  test/{ => libtest}/test.cpp |  0
>>  test/{ => libtest}/test.h   |  0
>>  test/meson.build            | 20 ++++++++++++++------
>>  4 files changed, 21 insertions(+), 6 deletions(-)
>>  create mode 100644 test/libtest/meson.build
>>  rename test/{ => libtest}/test.cpp (100%)
>>  rename test/{ => libtest}/test.h (100%)
>>
>> diff --git a/test/libtest/meson.build b/test/libtest/meson.build
>> new file mode 100644
>> index 000000000000..b998154dd8d3
>> --- /dev/null
>> +++ b/test/libtest/meson.build
>> @@ -0,0 +1,7 @@
>> +libtest_sources = files([
>> +    'test.cpp',
>> +])
>> +
>> +libtest = static_library('libtest', libtest_sources)
>> +
>> +libtest_includes = include_directories('.')
>> diff --git a/test/test.cpp b/test/libtest/test.cpp
>> similarity index 100%
>> rename from test/test.cpp
>> rename to test/libtest/test.cpp
>> diff --git a/test/test.h b/test/libtest/test.h
>> similarity index 100%
>> rename from test/test.h
>> rename to test/libtest/test.h
>> diff --git a/test/meson.build b/test/meson.build
>> index da0aea9678d1..50ec11853203 100644
>> --- a/test/meson.build
>> +++ b/test/meson.build
>> @@ -1,11 +1,19 @@
>> -libtest_sources = files([
>> -    'test.cpp',
>> -])
>> +subdir('libtest')
>>
>> -libtest = static_library('libtest', libtest_sources)
>> +test_libraries = [libcamera, libtest]
>> +
>> +test_includes_public = [
>> +  libtest_includes,
>> +  libcamera_includes,
>> +]
>> +
>> +test_includes_internal = [
> 
> That's not really 'internal' if it includes the public includes :)

It's both ... but it's a define which is created for use by the internal
tests.

Would you prefer to call this something else?

> 
>> +  test_includes_public,
>> +  libcamera_internal_includes,
>> +]
>>
>>  test_init = executable('test_init', 'init.cpp',
>> -                       link_with : libcamera,
>> -                       include_directories : libcamera_includes)
>> +                       link_with : test_libraries,
>> +                       include_directories : test_includes_public)
> 
> What about a test/tests/ directory? Or do we expect to add all tests
> under test/ itself? Eg. test/libcamera/media_device_test.cpp ?

Up to you.

If it's a single test - it can live here with a simple addition to the
arrays created in:

 "[PATCH] test: Use foreach iterators to simplify definitions"

Or - if you have a group of related tests, they should be defined as a
'suite'. For an example of this please see the patch posted :

 "[PATCH 2/2] lib: Add V4L2 Device object"

In particular - please note that it adds the 'suite: v4l2device' flag in
the test/v4l2device/meson.build.

Perhaps we might want to split public-api tests from internal-api tests
... but we can worry about that once we've got a public-api to test :)

--
Regards

Kieran


> 
> Thanks
>   j
>>
>>  test('Initialisation test', test_init)
>> --
>> 2.17.1
>>
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel@lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Dec. 31, 2018, 9:03 a.m. UTC | #3
Hi Kieran,

Thank you for the patch.

On Friday, 21 December 2018 10:13:07 EET Kieran Bingham wrote:
> Create a subdirectory to contain the libtest helper library.
> 
> Define two variables to clarify when tests are aimed at public or
> internal components.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  test/libtest/meson.build    |  7 +++++++
>  test/{ => libtest}/test.cpp |  0
>  test/{ => libtest}/test.h   |  0
>  test/meson.build            | 20 ++++++++++++++------
>  4 files changed, 21 insertions(+), 6 deletions(-)
>  create mode 100644 test/libtest/meson.build
>  rename test/{ => libtest}/test.cpp (100%)
>  rename test/{ => libtest}/test.h (100%)
> 
> diff --git a/test/libtest/meson.build b/test/libtest/meson.build
> new file mode 100644
> index 000000000000..b998154dd8d3
> --- /dev/null
> +++ b/test/libtest/meson.build
> @@ -0,0 +1,7 @@
> +libtest_sources = files([
> +    'test.cpp',
> +])
> +
> +libtest = static_library('libtest', libtest_sources)
> +
> +libtest_includes = include_directories('.')
> diff --git a/test/test.cpp b/test/libtest/test.cpp
> similarity index 100%
> rename from test/test.cpp
> rename to test/libtest/test.cpp
> diff --git a/test/test.h b/test/libtest/test.h
> similarity index 100%
> rename from test/test.h
> rename to test/libtest/test.h
> diff --git a/test/meson.build b/test/meson.build
> index da0aea9678d1..50ec11853203 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -1,11 +1,19 @@
> -libtest_sources = files([
> -    'test.cpp',
> -])
> +subdir('libtest')
> 
> -libtest = static_library('libtest', libtest_sources)
> +test_libraries = [libcamera, libtest]
> +
> +test_includes_public = [
> +  libtest_includes,
> +  libcamera_includes,
> +]
> +
> +test_includes_internal = [
> +  test_includes_public,
> +  libcamera_internal_includes,

Apart from the indentation issue,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +]
> 
>  test_init = executable('test_init', 'init.cpp',
> -                       link_with : libcamera,
> -                       include_directories : libcamera_includes)
> +                       link_with : test_libraries,
> +                       include_directories : test_includes_public)
> 
>  test('Initialisation test', test_init)

Patch

diff --git a/test/libtest/meson.build b/test/libtest/meson.build
new file mode 100644
index 000000000000..b998154dd8d3
--- /dev/null
+++ b/test/libtest/meson.build
@@ -0,0 +1,7 @@ 
+libtest_sources = files([
+    'test.cpp',
+])
+
+libtest = static_library('libtest', libtest_sources)
+
+libtest_includes = include_directories('.')
diff --git a/test/test.cpp b/test/libtest/test.cpp
similarity index 100%
rename from test/test.cpp
rename to test/libtest/test.cpp
diff --git a/test/test.h b/test/libtest/test.h
similarity index 100%
rename from test/test.h
rename to test/libtest/test.h
diff --git a/test/meson.build b/test/meson.build
index da0aea9678d1..50ec11853203 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -1,11 +1,19 @@ 
-libtest_sources = files([
-    'test.cpp',
-])
+subdir('libtest')
 
-libtest = static_library('libtest', libtest_sources)
+test_libraries = [libcamera, libtest]
+
+test_includes_public = [
+  libtest_includes,
+  libcamera_includes,
+]
+
+test_includes_internal = [
+  test_includes_public,
+  libcamera_internal_includes,
+]
 
 test_init = executable('test_init', 'init.cpp',
-                       link_with : libcamera,
-                       include_directories : libcamera_includes)
+                       link_with : test_libraries,
+                       include_directories : test_includes_public)
 
 test('Initialisation test', test_init)