Re: [PATCH 16/20] test-symtab: add tests for whitelisted functions

[Dodji Seketeli <dodji@seketeli.org>]

Hello,

Matthias Maennich <maennich@google.com> a écrit:

> diff --git a/tests/test-symtab.cc b/tests/test-symtab.cc
> index 2df766de1c5c..ac853e895fef 100644
> --- a/tests/test-symtab.cc
> +++ b/tests/test-symtab.cc
> @@ -11,11 +11,14 @@
>  
>  #include <iostream>
>  #include <limits>
> +#include <string>
>  #include <vector>
>  
>  #include "abg-corpus.h"
>  #include "abg-dwarf-reader.h"
> +#include "abg-fwd.h"
>  #include "abg-ir.h"
> +#include "abg-tools-utils.h"
>  #include "lib/catch.hpp"
>  #include "test-utils.h"
>  
> @@ -26,12 +29,16 @@ using dwarf_reader::read_context_sptr;
>  using dwarf_reader::read_corpus_from_elf;
>  using ir::environment;
>  using ir::environment_sptr;
> +using suppr::suppressions_type;
>  
>  static const std::string test_data_dir =
>      std::string(abigail::tests::get_src_dir()) + "/tests/data/test-symtab/";
>  
>  dwarf_reader::status
> -read_corpus(const std::string path, corpus_sptr& result)
> +read_corpus(
> +  const std::string&		  path,
> +  corpus_sptr&			  result,
> +  const std::vector<std::string>& whitelist_paths = std::vector<std::string>())

Please keep the open parenthesis of the on the same line as at least one
parameter.

There are also several other functions indented like this in the file,
it would be good to fix them too.

>  {
>    const std::string& absolute_path = test_data_dir + path;
>  
> @@ -41,6 +48,15 @@ read_corpus(const std::string path, corpus_sptr& result)
>        absolute_path, debug_info_root_paths, env.get(),
>        /* load_all_type = */ true, /* linux_kernel_mode = */ true);
>  
> +  if (!whitelist_paths.empty())
> +    {
> +      const suppressions_type& wl_suppr =
> +	tools_utils::gen_suppr_spec_from_kernel_abi_whitelists(
> +	  whitelist_paths);
> +      REQUIRE_FALSE(wl_suppr.empty());
> +      dwarf_reader::add_read_context_suppressions(*ctxt, wl_suppr);
> +    }
> +
>    dwarf_reader::status status = dwarf_reader::STATUS_UNKNOWN;
>    result = read_corpus_from_elf(*ctxt, status);
>  
> @@ -76,14 +92,17 @@ TEST_CASE("Symtab::NoDebugInfo", "[symtab, basic]")
>  #define N std::numeric_limits<size_t>::max()
>  
>  corpus_sptr
> -assert_symbol_count(const std::string& path,
> -		    size_t	       function_symbols = 0,
> -		    size_t	       variable_symbols = 0,
> -		    size_t	       undefined_function_symbols = 0,
> -		    size_t	       undefined_variable_symbols = 0)
> +assert_symbol_count(
> +  const std::string&		  path,
> +  size_t			  function_symbols = 0,
> +  size_t			  variable_symbols = 0,
> +  size_t			  undefined_function_symbols = 0,
> +  size_t			  undefined_variable_symbols = 0,
> +  const std::vector<std::string>& whitelist_paths = std::vector<std::string>())

Likewise.

>  {
>    corpus_sptr		     corpus_ptr;
> -  const dwarf_reader::status status = read_corpus(path, corpus_ptr);
> +  const dwarf_reader::status status =
> +    read_corpus(path, corpus_ptr, whitelist_paths);
>    REQUIRE(corpus_ptr);
>  
>    REQUIRE((status & dwarf_reader::STATUS_OK));
> @@ -188,6 +207,76 @@ TEST_CASE("Symtab::SimpleSymtabs", "[symtab, basic]")
>    }
>  }

[...]

> Extend the test functionality in test-symtab to allow processing of KMI
> whitelists and add additional test cases for whitelist handling.
>
> 	* tests/data/Makefile.am: add new test files
> 	* tests/data/test-symtab/basic/one_function_one_variable_all.whitelist: New test file,
> 	* tests/data/test-symtab/basic/one_function_one_variable_function.whitelist: Likewise.
> 	* tests/data/test-symtab/basic/one_function_one_variable_irrelevant.whitelist: Likewise.
> 	* tests/data/test-symtab/basic/one_function_one_variable_variable.whitelist: Likewise.
> 	* tests/test-symtab.cc (read_corpus): Add support for whitelists.
> 	(assert_symbol_count): Likewise.
> 	(Symtab::SymtabWithWhitelist): New testcase.
>
> Reviewed-by: Giuliano Procida <gprocida@google.com>
> Signed-off-by: Matthias Maennich <maennich@google.com>

OK to apply to master with the changes above after the prerequisite
patches are in.

Thanks!

[...]

Cheers,

-- 
		Dodji