[Mesa-dev] [PATCH 2/2] mesa: Add test for sorted extension table

[Ian Romanick]

On 11/18/2015 03:01 PM, Nanley Chery wrote:
> From: Nanley Chery <nanley.g.chery at intel.com>
> 
> Enable developers to know if the table's alphabetical sorting
> is maintained or lost.

I like this in principle, but let's be honest.  Almost all of the time,
the people who don't sort (by whatever order we agree) also don't run
'make check'. :(

> Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> ---
>  src/mesa/main/extensions.h              |  1 +
>  src/mesa/main/tests/Makefile.am         |  1 +
>  src/mesa/main/tests/mesa_extensions.cpp | 47 +++++++++++++++++++++++++++++++++
>  3 files changed, 49 insertions(+)
>  create mode 100644 src/mesa/main/tests/mesa_extensions.cpp
> 
> diff --git a/src/mesa/main/extensions.h b/src/mesa/main/extensions.h
> index 1615e1c..7114c96 100644
> --- a/src/mesa/main/extensions.h
> +++ b/src/mesa/main/extensions.h
> @@ -85,6 +85,7 @@ enum {
>  #define EXT(name_str, ...) MESA_EXTENSION_##name_str,
>  #include "extensions_table.h"
>  #undef EXT
> +   MESA_EXTENSION_COUNT
>  };

In partial response to Ilia's and Emil's feedback... this test doesn't
really care about extensions.h.  It cares about the data in
extensions_table.h.  You could instead include extensions_table.h
directly in the test.  Then you could define EXT() to be whatever you
want, use ARRAY_SIZE, etc.  That way if we changed the format of
_mes_extension_table (say, changed name to ExtensionName), we wouldn't
have to modify the test.

I don't have a strong opinion.

> diff --git a/src/mesa/main/tests/Makefile.am b/src/mesa/main/tests/Makefile.am
> index bd7ab73..d6977e2 100644
> --- a/src/mesa/main/tests/Makefile.am
> +++ b/src/mesa/main/tests/Makefile.am
> @@ -27,6 +27,7 @@ AM_CPPFLAGS += -DHAVE_SHARED_GLAPI
>  main_test_SOURCES +=			\
>  	dispatch_sanity.cpp		\
>  	mesa_formats.cpp			\
> +	mesa_extensions.cpp			\
>  	program_state_string.cpp
>  
>  main_test_LDADD += \
> diff --git a/src/mesa/main/tests/mesa_extensions.cpp b/src/mesa/main/tests/mesa_extensions.cpp
> new file mode 100644
> index 0000000..5154ae1
> --- /dev/null
> +++ b/src/mesa/main/tests/mesa_extensions.cpp
> @@ -0,0 +1,47 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +/**
> + * \name mesa_extensions.cpp
> + *
> + * Verify that the extensions table is sorted.
> + *

Delete the empty " *" line.

> + */
> +
> +#include <gtest/gtest.h>
> +#include "main/mtypes.h"
> +#include "main/extensions.h"
> +
> +/**
> + * Debug/test: verify the extension table is alphabetically sorted.
> + */
> +TEST(MesaExtensionsTest, AlphabeticallySorted)
> +{
> +   for (int i = 0; i < MESA_EXTENSION_COUNT - 1; ++i) {
> +      const char * current_str = _mesa_extension_table[i].name;
> +      const char * next_str = _mesa_extension_table[i+1].name;
> +
> +      /* We expect the extension table to be alphabetically sorted */
> +      ASSERT_LT(strcmp(current_str, next_str), 0);
> +   }
> +}
>