Opened 6 years ago

Closed 4 years ago

#12953 closed Feature Requests (fixed)

access to master_test_suite().{argc, argv}

Reported by: ope-devel@… Owned by: Raffi Enficiaud
Milestone: Boost 1.68.0 Component: test
Version: Boost 1.63.0 Severity: Problem
Keywords: Cc:

Description

Hello,

for testing I use a "loader" class to read input and expected files from filesystem. These files are used by the data-driven test cases. The path to these files are given from command line.

Due to the fact that datasets are creating static objects on file scope, I can't access them by calling master_test_suite().{argc, argv} outside of e.g. BOOST_AUTO_TEST_CASE where the "loader" exist.

Using the Fixture approach fails to compile, see attachement. Further, as far I understood, the files would be loaded again for each data driven test, which is not intended.

Following the docs, I didn't found a way to get access to the (in best case from boost.test args pruned) argc/argv. Is it a missing feature and bug?

The initial thread is on ML "[boost] [Boost.Test] access to boost::unit_test::framework::master_test_suite().{argc, argv} outside from BOOST_TEST"

Thanks, Olaf

Attachments (1)

bt_dataset_fixture.cpp (1.8 KB ) - added by anonymous 6 years ago.

Download all attachments as: .zip

Change History (18)

by anonymous, 6 years ago

Attachment: bt_dataset_fixture.cpp added

comment:1 by Raffi Enficiaud, 6 years ago

Thanks for the report, I believe it is better to continue this thread here :)

comment:2 by Raffi Enficiaud, 5 years ago

Owner: changed from Gennadiy Rozental to Raffi Enficiaud
Status: newassigned
Type: BugsFeature Requests

comment:3 by Raffi Enficiaud, 5 years ago

Hi there,

I have an idea for addressing this, would you have some bandwidth to test?

comment:4 by Raffi Enficiaud, 5 years ago

There is an implementation tentative in the branch topic/12953-access-master_test_suite-in-datatest-cases. It would be really helpful if you have some time to test the branch with your code.

comment:5 by Raffi Enficiaud, 5 years ago

Milestone: To Be DeterminedBoost 1.66.0

comment:6 by ope.devel@…, 4 years ago

Hi Raffi,

sorry for the later answer, I lost over the time this thread :-( Thanks for investigation but I assume it doesn't work yet (or I mis omething)

$ bin/bt_dataset_args -- test

ArgCount = 0 Mock ArgCount = 0 Running 1 test case...

Unfortuately I don't see something to add an attachement here, therefore I embedd my test code. The '--' comment line separate different translation units, if it matters. The loader is re-used several time for completly different tests.

Thanks, Olaf

//------------------------------------------------------------------------------

#define BOOST_TEST_MODULE "Cool Parser Test Suite"
#include <boost/test/included/unit_test.hpp>


//------------------------------------------------------------------------------

#include <boost/test/data/test_case.hpp>
#include <boost/filesystem/fstream.hpp>
#include <iostream>


namespace testsuite {

namespace fs = boost::filesystem;

struct app_mock
{
    int const                                       argc;
    char** const                                    argv;

    app_mock()
    : argc(boost::unit_test::framework::master_test_suite().argc)
    , argv(boost::unit_test::framework::master_test_suite().argv)
    { }
};


BOOST_AUTO_TEST_CASE( app_mocker )
{
     app_mock app;
}


class dataset_loader
{
public:
    typedef std::vector<fs::path>                       pathname_type;
    typedef std::vector<std::string>                    data_type;

public:
    pathname_type const& test_case_name() const { return m_test_case; }
    data_type const& input() const { return m_input; }
    data_type const& expect() const { return m_expected; }

public:
    dataset_loader(fs::path const& path);

private:
    void read_files(fs::path const& path);

private:
    pathname_type                                       m_test_case;
    data_type                                           m_input;
    data_type                                           m_expected;
    std::string                                         input_extension;
    std::string                                         expected_extension;
};

} // namespace testsuite



namespace testsuite {

dataset_loader::dataset_loader(fs::path const& path)
: input_extension{".input" }
, expected_extension{ ".expected" }
{
    BOOST_TEST_INFO("dataset_loader load test files from " << path);

    auto const argc{boost::unit_test::framework::master_test_suite().argc};
    auto const argv{boost::unit_test::framework::master_test_suite().argv};

    std::cout << "ArgCount = " << argc << '\n';
    for(unsigned i = 0; i != argc; i++) {
        std::cout << "ArgValue = " << argv[i] << '\n';
    }

    app_mock m;
    std::cout << "Mock ArgCount = " << m.argc << '\n';
    for(int i = 0; i != m.argc; i++) {
        std::cout << "Mock ArgValue = " << m.argv[i] << '\n';
    }

    read_files(path);
}


void dataset_loader::read_files(fs::path const& path)
{
    // real code suppressed, cmd line args for load prefix and extensions required
    BOOST_TEST_INFO("read files " << path << input_extension);
}


} // namespace testsuite

//------------------------------------------------------------------------------

#include <boost/test/unit_test.hpp>


BOOST_AUTO_TEST_SUITE( concrete_testsuite )

using ::boost::unit_test::data::monomorphic::operator^;


struct concrete_dataset : public testsuite::dataset_loader
{
    concrete_dataset()
    : dataset_loader{ "test_case/number_42" }
    { }
} const concrete_dataset;


BOOST_DATA_TEST_CASE(cool_test,
    concrete_dataset.input() ^ concrete_dataset.expect(),
    input, expect)
{
    // ...
    BOOST_TEST(input == expect);
}


BOOST_AUTO_TEST_SUITE_END()

comment:7 by Raffi Enficiaud, 4 years ago

Thanks for the reply. Have you tested the branch I mentioned or the develop/master branch? Thanks for posting the code, I will try on my end and keep you posted.

comment:8 by ope.devel@…, 4 years ago

Hello Raffi,

yes, same result applies to the branch you mentioned and 1.67.0

comment:9 by Raffi Enficiaud, 4 years ago

Thanks for testing. In fact, it could not work as it was, as the concrete_dataset is built already at the declaration of the data test case.

I made another declaration that creates a lazy evaluation of the dataset. In order to use it, you have to use a make_delayed construct instead. Please have a look at this:

https://github.com/boostorg/test/blob/topic/12953-access-master_test_suite-in-datatest-cases/test/test-organization-ts/dataset-master-test-suite-accessible-test.cpp#L80

on the same branch as the one mentioned before.

Only the zip operation over datasets is currently honouring this new construct, I will do the rest today. The test is adapted from yours as you can see.

Let me know if this does work for you.

Thanks, Raffi

comment:10 by ope-devel@…, 4 years ago

Hello Raffi,

thank your for your effort! But I'm not sure if I can adapted theses solution 1:1. In the real project I use the dataset as aggregate of test files and informations, the 'expected' file and as 3rd field the test case file name:

class dataset_loader
{
public:
    typedef std::vector<fs::path>                       pathname_type;
    typedef std::vector<std::string>                    data_type;

public:
    pathname_type const& test_case_name() const { return m_test_case; }
    data_type const& input() const { return m_input; }
    data_type const& expect() const { return m_expected; }

public:
    dataset_loader(fs::path const& path,
        std::string const& relative_path,   // relative path from testsuite 
        std::string const& input_extension  // test case file input file extension
    );

private:
    void read_files(fs::path const& path);
    std::string read_file(fs::path const& file_path);

private:
    fs::path                                            m_prefix_dir;
    pathname_type                                       m_test_case;
    data_type                                           m_input;
    data_type                                           m_expected;
    std::string                                         input_extension;
    std::string                                         expected_extension;
};

The class checks if there is for each 'input' file a complementary 'expect' file, otherwise the input is discarded. This allows me to skip specific tests by only simply renaming the input file. Your approach creates in my use case for the test bench 2 (isn't it?) objects to load the files. At zip time I get an error if the element numbers doesn't match. Hence, I have to take care to rename the 'expect' file too - which is a burden and error prone. Further, as far I see, no way to get the case name (not right, I see a long path from boost.test IIRC). The failure test output e.g. "/_0" gives an enumeration id, with test case name I can specify the concrete file name, see notes on bottom.

Here is what happens under the hood (error handling not shown :-)

dataset_loader::dataset_loader(fs::path const& path,
    std::string const& relative_path,
    std::string const& input_extension_
)
: m_prefix_dir{ CMAKE_TESTSUITE_PREFIX_READ_PATH } // => argv [1]
, input_extension{input_extension_} // => argv[2]
, expected_extension{ ".expected" } // => argv[3]
{
    BOOST_TEST_INFO("dataset_loader load test files from " << path);
    fs::path p = m_prefix_dir / fs::path(relative_path) / path;
    read_files(p);
    assert(m_input.size() == m_expected.size()
           && "dataset_loader test vector size mismatch");
}

void dataset_loader::read_files(fs::path const& path)
{
    try {
        if(fs::exists(path) && fs::is_directory(path)) {

            std::vector<fs::path> dir_list { };
            std::copy(fs::directory_iterator(path),
                      fs::directory_iterator(),
                      std::back_inserter(dir_list));

            for(auto const& file : dir_list) {
                if (fs::extension(file) == input_extension) {
                    m_test_case.emplace_back(
                        file.parent_path().filename() / file.stem()
                    );
                    fs::path const input_file  = file;
                    fs::path const expect_file = fs::change_extension(file, expected_extension);
                    m_input.emplace_back(   read_file(input_file ));
                    m_expected.emplace_back(read_file(expect_file));
                }
            }
        }
        else { /* error message */ }
    }
    catch(std::exception const& e) { /* ... */ }
}

std::string dataset_loader::read_file(fs::path const& file_path)
{
    fs::ifstream file{ file_path };
    std::ostringstream ss{};
    ss << file.rdbuf();
    return ss.str();
}

where the test case directory structure is:

$ROOT
  /test_case
    /concrete_test
      case_a_000.{input,expected}
      case_a_001.{input,expected}
      case_b_000.{input,expected}
      case_b_001.{input,expected}
    /other_concrete_test
      ....
  .. ~ 150 files each of input,expected

and the test case self:

struct foo_dataset : public testsuite::dataset_loader
{
    foo_dataset()
    : dataset_loader{ "test_case/parse_identifier",
                      "../parse/syntax",
                      ".pls" }
    { }
} const foo_dataset;

BOOST_DATA_TEST_CASE( basic_syntax,
      foo_dataset.input()
    ^ foo_dataset.expect()
    ^ foo_dataset.test_case_name(),
    input, expected, test_case_name)
{
   ...
}

// ... and > 100 BOOST_DATA_TEST_CASE files with different path and relative_path
// constructor args

Imo, using the solution you provided would result into 2 dataset_loader instances, loading 'input' and 'expected'. How to get the bullet-proof features (filter missing paired test cases at load time and test_case name) using it?

Thank you, Olaf

comment:11 by Raffi Enficiaud, 4 years ago

Hi Olaf,

Thank you for your precise reply. The keys for adapting those developments to your problem are:

  • you should instanciate a dataset, otherwise it gets ... instanciated at in the global scope we do not have access to the master_test_suite argc and argv. This is what happens with your foo_dataset
  • to ensure that the returned elements are all consistent, you can play with the arity of the dataset. So instead of creating 2/3 as you do, you just create one that returns tuples. Those tuples will get expanded automatically.
  • the zip size constraint is happening at the access to the dataset. If this is constructed lazylly as in the branch, this access is delayed.

Here is an adaptation of your problem to the new code:

class dataset_loader_arity3
{
public:
    typedef std::vector<std::string>                    data_type;

    data_type m_expected;
    data_type m_input;
  
    typedef std::string sample;
    enum { arity = 3 };

public:
    dataset_loader_arity3(std::string some_additional) : m_some_additional(some_additional)
    {
      int argc = boost::unit_test::framework::master_test_suite().argc;
      char** argv = boost::unit_test::framework::master_test_suite().argv;

      
      for(unsigned i = 1; i != argc; i++) {
        std::string current(argv[i]);
        std::cout << "current " << current << std::endl;
        if(current.find("--param1") != std::string::npos) {
          m_expected.push_back(current);
        }
        else {
          m_input.push_back(current);
        }
      }
    }
  
    struct iterator {
        iterator(
          data_type::const_iterator v_expected,
          data_type::const_iterator v_input,
          std::string additional)
        : m_input(v_input)
        , m_expected(v_expected)
        , m_additional(additional)
        {}

        // bug in joins, see 13380. We should return a non temporary
        std::tuple<std::string, std::string, std::string> operator*() const {
          return std::tuple<std::string, std::string, std::string>(*m_input, *m_expected, *m_input + " -" + m_additional + "- " + *m_expected);
        }
        void operator++()
        {
            ++m_input;
            ++m_expected;
        }
    private:
        data_type::const_iterator m_input, m_expected;
        std::string m_additional;
    };
  
    boost::unit_test::data::size_t size() const {
      return m_input.size();
    }

    // iterator
    iterator        begin() const   { return iterator(m_expected.begin(), m_input.begin(), m_some_additional); }
  
private:
    std::string m_some_additional;

};

namespace boost { namespace unit_test { namespace data {

namespace monomorphic {
  template <>
  struct is_dataset<dataset_loader_arity3> : boost::mpl::true_ {};
}
} } }

BOOST_DATA_TEST_CASE(master_access_make_ds_with_arity,
    boost::unit_test::data::make_delayed<dataset_loader_arity3>( "something-in-the-middle"),
    input, expected, additional)
{
    std::cout << "input: " << input << " -- expected: " << expected << " -- additional: " << additional << std::endl;
    BOOST_TEST(true);
}

You can see in master_access_make_ds_with_arity that 3 variables are given to the test case. I construct 2 of them from the command line, and the third is created on the fly. This last one depends on a static/global variable in the example, that is passed to the ctor of the dataset when needed.

All the magic is happening in the iterator of the dataset. I think this example is relatively close to yours. Let me know if you have any further question, but I think on my side, the ticket is more or less addressed :)

Best, Raffi

comment:12 by ope-devel@…, 4 years ago

Hello Raffi,

thanks a lot; master_access_make_ds_with_arity solves it in the manner I considered to use before.

Is it correct, that inside the BOOST_DATA_TEST_CASE I've access to argc/argv anywhere?

The use case is further using an own 'reporter':

BOOST_DATA_TEST_CASE( foo,
      foo_dataset.input()
    ^ foo_dataset.expect()
    ^ foo_dataset.test_case_name(),
    input, expected, test_case_name)
{
    ...
    testing_parser<attribute_type> parse;
    auto [parse_ok, parse_result] = parse(input, parser, test_case_name);

    BOOST_TEST(parse_ok);
    BOOST_REQUIRE_MESSAGE(parse_ok,
        report_diagnostic(test_case_name, input, parse_result)
    );

    BOOST_TEST(parse_result == expected, btt::per_element());
    BOOST_REQUIRE_MESSAGE(current_test_passing(),
        report_diagnostic(test_case_name, input, parse_result)
    );
}

where

std::string report_diagnostic(
    fs::path const& test_case_name,
    std::string const& input,
    std::string const& result
)
{
    ...
    // only write in case of failed test
    if(!current_test_passing()) {
        test_case_result_writer result_writer(test_case_name);
        result_writer.write(result);
    }
    return ss.str();
}

where the path to be written (is hardcoded at this time) shall be given as argv too.

struct test_case_result_writer 
{
test_case_result_writer(fs::path const& test_case)
: m_dest_dir{ CMAKE_BT_TEST_CASE_WRITE_PATH } // ARGV[4]
, m_test_case{ test_case }
{ /* FixMe: Gather write path 'm_dest_dir' from argv */}
....

void write(std::string const& parse_result)
{
    fs::path const full_pathname = m_dest_dir / "test_case" / m_test_case;
    fs::path const write_path = full_pathname.parent_path();
    std::string const ext{ ".parsed" };

    if(!create_directory(write_path)) { ... } 
    fs::path const filename = full_pathname.filename().replace_extension(ext);
    write_file(write_path / filename, parse_result);
}

So the dataset_loader can be used independing of what to check/load.

One point using dataset-master-test-suite-accessible-test.cpp:

> bin\bt_issue12953.exe -- --param1=1 --param2=2
Running 22 test cases...

*** No errors detected

but:

bin\bt_issue12953.exe --list_content
Test setup error: Can't zip datasets of different sizes

How to handle this?

Best, Olaf

PS: As you can see current_test_passing() serves arround another problem not related to the feature request.

comment:13 by Raffi Enficiaud, 4 years ago

Hi Olaf,

So yes, once the test starts (in BOOST_DATA_TEST_CASE) you have access to the argv as usual. The only issue you were facing is that argv should be used in your setup to create the tests and it was not supported. If you need to pass an additional parameter to the test for the reporting you have, I would rather suggest to access it through the master_test_suite_t argv. The other option would be to change the iterator of the dataset to return this parameter. In the example I posted, there is a third parameter that is given to the test.

You can get the current test case name directly from the framework. See here for instance.

For the error raised by passing --list_content, this is just a problem in handling the datasets, not a boost.test issue (I got it running for the example I gave you by disabling all the other tests from the file test/test-organization-ts/dataset-master-test-suite-accessible-test.cpp in this branch).

Best, Raffi

comment:14 by Raffi Enficiaud, 4 years ago

Milestone: Boost 1.66.0Boost 1.68.0

comment:15 by anonymous, 4 years ago

Thank you Raffi,

I think, the feature request is adressed. With respect to the error raised by passing -- list_content, I open another issue or write to the ML, isn't it?

Just seen, I'm looking forward to Milestone 1.68.0

Thanks, Olaf

comment:16 by Raffi Enficiaud, 4 years ago

Hi,

It is in master now. Should be available for 1.68. Concerning --list_content, as I said I am not reproducing it on my end. In all case, better create another ticket.

Cheers, Raffi

comment:17 by Raffi Enficiaud, 4 years ago

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.