Incorrect handling of test cases with multiple dots in the name.

[ftrofin]

Thanks to Christian's help I am now able to see my tests in Test Explorer! One issue I uncovered though is that some tests which have a more "hierarchical" name using multiple dots in the name don not work properly. Here is an example:

When I list my tests using my test executable and --gtest_list_tests I get something like:

FactorialTests.
testFactorialInline
testFailureWithRequire
OtherTests.TestKeychain.
TestPassword

These show up like this in Test Explorer:

FactorialTests.testFactorialInline
FactorialTests.testFailureWithRequire
OtherTests.TestPassword

Notice that the "OtherTests.TestKeychain." suite name became only "OtherTests." which causes this test not to be found when I try to run it individually (because there is no registered test with "OtherTests.TestPassword" name and gtest filter returns empt list) The first two tests run fine.

The issue seems to be ParseListTestsOutput in Core/TestCases/ListTestsParser.cs. Can you please take a look and confirm the bug?

Thanks!

Florin

[csoltenborn]

Hi Florin, thanks for the detailed bug report and the great analysis! (btw, in case you keep looking at our code: we are currently working on branch #74_ProjectSpecificSettings)

However, I'm not able to reproduce this behavior since I cannot create a compiling example test. Here's what I get:

This is actually not surprising since the Google Test macros transform each test "method" into a class, the name of which contains both suite name (or fixture name) and test name, and C++ identifiers must not contain dots. Can you please give me a hint (or even better: provide some example code)?

[ftrofin]

You're right, you can't create them with the macros directly. But you can create them via the gtest API. Which is what I'm doing. Long story short: we have a bunch of legacy unit tests and want to switch to gtest. To preserve some existing functionality, I wrote some code that iterates the legacy tests and registers them with gtest. Those tests are in a hierarchy (hence the dots in the name) and gtest doesn't have a problem with that (the TEST macro resolves to a call to the MakeAndRegisterTestInfo() function which is what I call directly from my adaptor).

So, yes, I agree that this is probably not a relevant test case for the majority of your users but I am sure that there are others out there that need to integrate their legacy testing frameworks with gtest and will trip over the same issue. I suspect that the fix would not be hard to make, the regex used for extracting the test case name would have to be adjusted to be more greedy and not stop at the first dot but at the last.

Thanks!

[csoltenborn]

Ah, I see... Could you still provide some small example? We haven't seen that approach so far, and we'd like to a) learn how this can be done and b) we need code to write unit tests against before doing that change (which we are of course willing to do even if it indeed sounds like a corner case, as long as it's not breaking support for the usual way of writing unit tests)...

If you have the time for that, please integrate that code into our SampleTests solution and provide a pull request. Otherwise, feel free to attach the code to this issue, we will then port it into our SampleTests solution.

[ftrofin]

The code below should give you an idea of what we're doing (I tried to keep this small and relevant):

We have an adapter that uses the Visitor pattern to iterate out legacy tests. Inside the VisitSelf we do:

// inTestCase is a pointer to a legacy test case
void VisitSelf(dvatesting::TestCaseBase* inTestCase) {
        // ... Constructs the suiteName and testName strings from the legacy test hierarchy.
        // suiteName will be something like "OtherTests.TestKeychain" - notice the dot
        // and testName is "TestPassword".

        // Register the test case with gtest
        ::testing::internal::MakeAndRegisterTestInfo(
                                                     suiteName.c_str(),
                                                     testName.c_str(), NULL, NULL,
                                                     ::testing::internal::CodeLocation(__FILE__, __LINE__),
                                                     ::testing::internal::GetTestTypeId(),
                                                     ::testing::Test::SetUpTestCase,
                                                     ::testing::Test::TearDownTestCase,
                                                     new TestFactoryAdapter(inTestCase));
}

Where TestFactoryAdapter() is defined as:

// Adapter that bridges the legacy test classes and gtest
// Creates an instance of the test fixture
class TestFactoryAdapter : public testing::internal::TestFactoryBase {
    dvatesting::TestCaseBase* mDVATestCase; // Non-owning pointer
public:
    TestFactoryAdapter(dvatesting::TestCaseBase* dvaTestCase) : mDVATestCase(dvaTestCase){
    }
    virtual ::testing::Test* CreateTest() { return new DVATestWrapper(mDVATestCase); }
};

This simply creates a test wrapper that invokes our legacy's test entry point when gtest wants to run it (see TestBody() below):

class DVATestWrapper: public ::testing::Test  {
    dvatesting::TestCaseBase* mDVATestCase; // our legacy test instance
public:
    DVATestWrapper(dvatesting::TestCaseBase* dvaTestCase): mDVATestCase(dvaTestCase) {
    }
protected:
    virtual void TestBody() {
        mDVATestCase->Run(); // Calls our legacy test entry point
    }
    virtual void SetUp() {}
    virtual void TearDown() {}
    static void SetUpTestCase() {}
    static void TearDownTestCase() {}
};

I'm sorry I don't have more time to write an actual test! I hope this is enough information to repro the case on your side. You don't need the visitor stuff, just call the MakeAndRegisterTestInfo from a file-scoped global variable like this:

testing::TestInfo* customTestName = ::testing::internal::MakeAndRegisterTestInfo(
                                                     "Custom.Suite.Name",
                                                     "CustomTestName", NULL, NULL,
                                                     ::testing::internal::CodeLocation(__FILE__, __LINE__),
                                                     ::testing::internal::GetTestTypeId(),
                                                     ::testing::Test::SetUpTestCase,
                                                     ::testing::Test::TearDownTestCase,
                                                     new TestFactoryAdapter(inTestCase));

Please let me know if this helped or not! Many thanks again for all the help and support you've been given so far!

F.

[csoltenborn]

Thanks for the example code! I have your example running and can confirm the problem with the suite names. However, there seems to be no CodeLocation() method, and the MakeAndRegisterTestInfo() method does not take a file location (so using the existing FormatFileLocation() method wasn't the solution). I assume that you have coded this off your head - can you clarify if you are indeed passing the test location, and how you do it?

Which leads to another problem: If I use a suite name without dots, I can run the tests, but I do not get source file locations. This is not surprising since we get those locations from the test executable's pdb, where we look for symbol patterns derived from the suite and test names (which we receive by listing the according tests using the --gtest_list_tests option). Can you confirm that you do not have source file locations for your tests?

I do not see a straight-forward solution for the latter problem - you probably would have to live with that. Other problems are:

• Links to failing assertions point to the failed assertion in the TestWrapper class
• SCOPED_TRACEs do not appear to be working (haven't looked closer into this)

I have attached the source file I created so far for reference (it's probably going to be part of the Tests project of the SampleTests solution - ending .txt because GitHub doesn't support .cpp attachments).

ApiCreatedTests.txt

[ftrofin]

Hmm, something is wrong: on my computer, gtest-internal.h defines MakeAndRegisterTestInfo like this:

GTEST_API_ TestInfo* MakeAndRegisterTestInfo(
    const char* test_case_name,
    const char* name,
    const char* type_param,
    const char* value_param,
    CodeLocation code_location,
    TypeId fixture_class_id,
    SetUpTestCaseFunc set_up_tc,
    TearDownTestCaseFunc tear_down_tc,
    TestFactoryBase* factory);

Notice the CodeLocation (that data structure is also defined in that header). I think I have version 1.7.

Yes, some of the features will not work with the adapter (the failed assertion location being one as you noticed) but I am fine with that at the moment. This wrapper/bridge is intended as a short term band-aid while we port the legacy test over.

[csoltenborn]

We do not deliver gtest-internal.h with our SampleTests solution - that's why I didn't see it. And it shouldn't make a difference anyways, since we need to get code locations at test discovery time, and Google Test does not provide them when listing tests.

This version of GTA should work with dots in the suite names - feel free to give it a shot. Note that there are quite a few other changes in that version (see release notes), and that it's not heavily tested despite the automatic tests - your mileage may vary.

[ftrofin]

Thank you Christian! I downloaded 0.8.0.571 and I verified that dots now work in suite names. You are awesome!