Add support for named parameterised tests

[wasabinza]

Thanks for this project, it is really great.
I've extended my google test to use parameterised tests. If I use the default naming where the tests are identified numerically then they work, however if I specify a name generation function they work in google test outside of visual studio but this extension fails to accept the name.

The following errors and warnings occured during test execution (enable debug mode for more information):
ERROR: Exception while running tests: System.AggregateException: One or more errors occurred. ---> System.ArgumentException: The path is not of a legal form.
at System.IO.Path.LegacyNormalizePath(String path, Boolean fullCheck, Int32 maxPathLength, Boolean expandShortPaths)
at System.IO.Path.NormalizePath(String path, Boolean fullCheck, Int32 maxPathLength, Boolean expandShortPaths)
at System.IO.Path.GetFullPathInternal(String path)
at GoogleTestAdapter.TestCases.TestCaseFactory.NewCreateTestcases(Action1 reportTestCase, List1 standardOutput)
at GoogleTestAdapter.TestCases.TestCaseFactory.CreateTestCases(Action1 reportTestCase) at GoogleTestAdapter.GoogleTestDiscoverer.GetTestsFromExecutable(String executable) at GoogleTestAdapter.TestAdapter.TestExecutor.<>c__DisplayClass18_0.<AddTestCasesOfExecutable>b__0() at GoogleTestAdapter.Settings.SettingsWrapper.ExecuteWithSettingsForExecutable(String executable, Action action, ILogger logger) at GoogleTestAdapter.TestAdapter.TestExecutor.AddTestCasesOfExecutable(List1 allTestCasesInExecutables, String executable, SettingsWrapper settings, ILogger logger, Func1 testrunIsCanceled) at GoogleTestAdapter.TestAdapter.TestExecutor.<>c__DisplayClass17_1.<GetAllTestCasesInExecutables>b__2() at System.Threading.Tasks.Task.InnerInvoke() at System.Threading.Tasks.Task.Execute() --- End of inner exception stack trace --- at System.Threading.Tasks.Task.WaitAll(Task[] tasks, Int32 millisecondsTimeout, CancellationToken cancellationToken) at GoogleTestAdapter.Helpers.Utils.SpawnAndWait(Action[] actions, Int32 timeoutInMs) at GoogleTestAdapter.TestAdapter.TestExecutor.GetAllTestCasesInExecutables(IEnumerable1 executables)
at GoogleTestAdapter.TestAdapter.TestExecutor.TryRunTests(IEnumerable1 executables, IRunContext runContext, IFrameworkHandle frameworkHandle) at GoogleTestAdapter.TestAdapter.TestExecutor.RunTests(IEnumerable1 executables, IRunContext runContext, IFrameworkHandle frameworkHandle)
---> (Inner Exception #0) System.ArgumentException: The path is not of a legal form.
at System.IO.Path.LegacyNormalizePath(String path, Boolean fullCheck, Int32 maxPathLength, Boolean expandShortPaths)
at System.IO.Path.NormalizePath(String path, Boolean fullCheck, Int32 maxPathLength, Boolean expandShortPaths)
at System.IO.Path.GetFullPathInternal(String path)
at GoogleTestAdapter.TestCases.TestCaseFactory.NewCreateTestcases(Action1 reportTestCase, List1 standardOutput)
at GoogleTestAdapter.TestCases.TestCaseFactory.CreateTestCases(Action1 reportTestCase) at GoogleTestAdapter.GoogleTestDiscoverer.GetTestsFromExecutable(String executable) at GoogleTestAdapter.TestAdapter.TestExecutor.<>c__DisplayClass18_0.<AddTestCasesOfExecutable>b__0() at GoogleTestAdapter.Settings.SettingsWrapper.ExecuteWithSettingsForExecutable(String executable, Action action, ILogger logger) at GoogleTestAdapter.TestAdapter.TestExecutor.AddTestCasesOfExecutable(List1 allTestCasesInExecutables, String executable, SettingsWrapper settings, ILogger logger, Func`1 testrunIsCanceled)
at GoogleTestAdapter.TestAdapter.TestExecutor.<>c__DisplayClass17_1.b__2()
at System.Threading.Tasks.Task.InnerInvoke()
at System.Threading.Tasks.Task.Execute()<---

I tried the microsoft collaboration version "Test Adapter for Google Test" and it is able to enumerate the tests but it has numerous other problems in my use case of not being able to read the pdbs properly which may lead to why it fails to link to source properly.

I'm confused about which adapter is the future, should I persist in trying to get the collaboration version working or log bugs about this version that are fixed in the other one? I wonder if it would be possible to port the support for generated names from the "Test Adapter for Google Test" to this adapter?

Here is the link to the google test tests for the generated names:
https://github.com/google/googletest/blob/master/googletest/test/gtest-param-test_test.cc

For example this excerpt from these tests generates the above error:

class CustomFunctorNamingTest : public TestWithParamstd::string {};
TEST_P(CustomFunctorNamingTest, CustomTestNames) {}

struct CustomParamNameFunctor {
std::string operator()(const ::testing::TestParamInfostd::string& info) {
return info.param;
}
};

INSTANTIATE_TEST_CASE_P(CustomParamNameFunctor,
CustomFunctorNamingTest,
Values(std::string("FunctorName")),
CustomParamNameFunctor());

INSTANTIATE_TEST_CASE_P(AllAllowedCharacters,
CustomFunctorNamingTest,
Values("abcdefghijklmnopqrstuvwxyz",
"ABCDEFGHIJKLMNOPQRSTUVWXYZ",
"01234567890_"),
CustomParamNameFunctor());

[wasabinza]

I would be happy to try and build a branch with this fixed, if this is the way forward.

[csoltenborn]

Thanks for your feature request, and for offering to contribute that feature!

The relation between our and the MS test adapter is supposed to be as follows: We will implement new features within this repositoy; after releasing and stabilizing the features, they will be merged into the MS adapter (which is delivered as part of VS).

We are currently in an intermediate state: Lukasz (the MS developer taking care of the MS adapter) is in the process of merging our 0.10.1 version into their adapter (which has new features such as GTest project and file templates). As soon as this work is finished, we will merge that state into our develop branch and release. From that point on, we will proceed as described above. Note that this is not yet set in stone, but that's what we are currently planning.

Thus, I indeed think it's a good idea to port your patch to our adapter and make a pull request against our repository. However, I think that this will be easier for you and for us if you wait until we have merged the MS stuff. Therefore, my suggestion is to give us a couple of weeks until the above is done, and then see how we can integrate your work. Does that make sense to you?

As a side note: If you do not want to wait, feel free to do a pull request against our repository as you like - you then get CI for free, including a VSIX. We would "ignore" your pull request until the merge has been completed, though, and it would be up to you to update your pull request accordingly after the merge. Could still be useful, though - I will certainly have a look at your work and might already be able to provide some (more or less :-) ) helpful comments...

[wasabinza]

So I had two issues really compounding each other.
When I used a named test then the source code cannot be found, this triggered the attempt to load the pdb for the dependent libraries and I had a library that was linked with debugging information but the pdb file is not available. This caused a null dereference exception and stopped the test case loading.
This is fixed by checking for null before dereferencing the pointer in PeParser.cs
if (dbgDir != null && dbgDir->SizeOfData > 0)
This enables my tests to show up, but the source link is missing.

If I paste the google test example of a named parameter test into the SampleTests ParameterizedTests.cpp then it also fails to find the source.
The IsParamRegex specifies digits only at the end of the Regex. However if I change it to make it more general (I think it should be \w) then lots of tests that didn't previously match the Suite or Name regex suddenly do.

I will commit some things to my fork.

[wasabinza]

I tried to make the SuiteRegex or NameRegex only match if the TestMarkers are found by removing the last ? and then allowing \w for the final part of the IsParamRegex however this also mismatches some of the test names.

[csoltenborn]

I see... thanks already for your debugging work and the pull request!

I will today try to fix our CI build (which seems to have issues), but I will only have time to look at the issues themselves at the weekend. Could you in the meantime change your pull request such that it's against our develop branch? This is where we are going to merge the MS contributions into, and from we will produce the next release...