search

csoltenborn / GoogleTestAdapter

Visual studio extension that adds support for the C++ testing framework Google Test.
Other
142 stars 101 forks source link

Add support for managed code / C# #274

Closed tapika closed 4 years ago

tapika commented 5 years ago
csoltenborn commented 5 years ago

Thanks for your pull request! Please allow me a couple of days to have a look at it (I'm rather busy right now) - would you mind to in the meantime make sure that your PR passes CI?

tapika commented 5 years ago

Thanks for your pull request! Please allow me a couple of days to have a look at it (I'm rather busy right now) - would you mind to in the meantime make sure that your PR passes CI?

I've switched to my own batch, but I doubt that next error was caused by my changes:

OpenCover.Console.exe : Test Run Failed. At line:4 char:1

Are you familiar with problem like this ?

csoltenborn commented 5 years ago

I would assume that this is caused by the two failed tests... Maybe OpenCover gets confused by that - I have always focused on the failing tests and believe that the OpenCover error message will go away if they pass.

Curious about why you provided your own batch, but looking into it in detail will have to wait for a couple of days. I will however try to give support while you are trying to fix the CI (as I do now :-) )

codecov[bot] commented 5 years ago

Codecov Report

Merging #274 into master will decrease coverage by 0.4%. The diff coverage is 68.35%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #274      +/-   ##
==========================================
- Coverage   73.87%   73.46%   -0.41%     
==========================================
  Files         100       99       -1     
  Lines        3793     3810      +17     
  Branches      553      558       +5     
==========================================
- Hits         2802     2799       -3     
- Misses        813      829      +16     
- Partials      178      182       +4
Flag Coverage Δ
#Integration 57.42% <64.55%> (-0.32%) :arrow_down:
#Unit 58.81% <68.35%> (-0.35%) :arrow_down:
Impacted Files Coverage Δ
...estAdapter/Core/TestResults/XmlTestResultParser.cs 93% <ø> (-1%) :arrow_down:
...r/VsPackage.Shared/Debugging/VsDebuggerAttacher.cs 0% <0%> (ø) :arrow_up:
...oogleTestAdapter/Core/TestCases/ListTestsParser.cs 100% <100%> (ø) :arrow_up:
GoogleTestAdapter/DiaResolver/PeParser.cs 94.82% <100%> (+0.09%) :arrow_up:
GoogleTestAdapter/Core/Model/TestCase.cs 93.1% <100%> (+1.79%) :arrow_up:
...TestAdapter/Core/TestResults/ErrorMessageParser.cs 89.62% <15.38%> (-10.38%) :arrow_down:
...Adapter/Core/TestCases/StreamingListTestsParser.cs 83.33% <59.09%> (-12.02%) :arrow_down:
...stAdapter/Core/TestCases/MethodSignatureCreator.cs 92.85% <83.33%> (ø) :arrow_up:
...oogleTestAdapter/Core/TestCases/TestCaseFactory.cs 95.13% <88.23%> (-1.42%) :arrow_down:
csoltenborn commented 5 years ago

Sweet :-)

tapika commented 5 years ago

Do you have an e-mail or forum, where I could ask more stupid questions about Google test adapter. I think I have tried to send you couple of mails to university account, but without any result.

It would make sense to have a discussion forum at least, as answer could be used later on in future by others as well.

csoltenborn commented 5 years ago

University account? OMG :-) - I'm not actively reading that account any more, and it's probably not even associated to GTA, so how did you find out?

Anyways: If your questions are about this PR, let's discuss them here. For more general questions, feel free to open a new GitHub issue (I have a question tag). Some OS projects use StackOverflow and ask their users to flag questions with a certain tag which they actively monitor, but I figured that there are not that many questions that this is worth it, and GitHub issues should be found by Google, too.

tapika commented 5 years ago

If your questions are about this PR

PR stands for Production release - so when my changes will reach official release ?

What I have briefly tested - tests / Run Tests / Debug sometimes might not attach to process, but this happens only once, when starting for second time, Debug attach always works. Also based on call stacks information I suspect it does not relate to Google test adapter, but to Visual studio test platform itself. But this problem persist really rarely and immediately disappears, so it's not a problem in itself. Need to raise to VS Test platform.

For C# based testing I have implemented basic [TestClass] / [TestMethod] support (since some of our old tests uses VS Tests), but want to discover bit deeper how new kind of testing could be constructed for C# script based testing, and basically I could like to mirror file system in test explorer - so namespaces / class name would be replaced with folder names, with recursively long path, and individual tests would be replaced with file path / maybe method names.

I have briefly checked that recursive long subfolders list is not supported at the moment by Google test adapter, maybe namespace.class name kind of depth is supported but no longer than 1-2 levels before reaching test. Wondering if VSTest platform does supports this ?

csoltenborn commented 5 years ago

PR means Pull Request on GitHub...

Thanks a lot for your work! In fact, there's even a bit too much of it right now - let's tackle this step by step:

Thus, let's first figure which issues you are tackling, and then agree on how to get them merged, ok? Concerning the when: I'm planning a release within the next couple of weeks which will of course contain whatever we got merged into develop at that point in time.

C# based testing: If I understand correctly, then you want GTA to also handle tests written in C# with the MS test framework. Why is this? After all, VS comes with built-in support for such tests; plus, the VS test framework is designed such that different adapters handle different kinds of tests and can be installed in parallel (e.g. it's possible to run gtest and boost tests by installing both adapters), so why make GTA support these tests, too? Please explain....

Build script: it appears that you have introduced a more robust way to figure out the location of VS, but why change the file name? Please apply the changes to the existing file such that the differences can be seen more easily.

TestAdapterStarter: What's the purpose of this project?

TestCaseDescriptor: Is there a particular reason for removing that class, or is this a refactoring for the sake of simplifying the code? In case it's the latter, this might be a good idea, but should then be merged separately from the feature work.

Despite that, I have noticed two more things:

Concerning your questions/remarks:

tapika commented 5 years ago

Please raise pull requests against my develop branch

Currently develop & master branch codes are identical, but just to push changes in right direction, right ?

C# based testing: If I understand correctly, then you want GTA to also handle tests written in C# with the MS test framework. Why is this? After all, VS comes with built-in support for such tests; plus, the VS test framework is designed such that different adapters handle different kinds of tests and can be installed in parallel (e.g. it's possible to run gtest and boost tests by installing both adapters), so why make GTA support these tests, too? Please explain....

There are multiple problems in overall. Besides VS Test framework being unstable and buggy, there are other issues as well - for example ClassInitialize / ClassCleanup design failure (https://stackoverflow.com/a/24131990/2338477), and I guess most important from my perspective - if we want to drive in user interface testing, VS Test framework starts from non-main thread, and switching to main thread is not possible without coding your own test adapter.

Also in future I want to drive in testing from perspective "application tests itself", and this would mean that we do have some sort of "testing framework" inside our own application - then it will come into question - do I drag whole VS Test platform in or some assemblies from it, or do I just re-implement everything on my own. Now concluded with re-implementing, as whole VS Test Platform seems to be kinda heavy stuff.

Previously I have provided only GTA itself, but for example you have for google test starter it's own git repository - https://github.com/google/googletest - for C# we don't yet have, so I'm concluded adding necessary files into GTA git itself. I have coded now two examples:

1. https://github.com/tapika/TestAdapterForGoogleTest/tree/master/GoogleTestAdapter/TestCSharpClassic https://github.com/tapika/TestAdapterForGoogleTest/tree/master/GoogleTestAdapter/CSharpTestBootstrap

2. https://github.com/tapika/syncProj/blob/master/syncProjUnitTesting.cs https://github.com/tapika/syncProj/blob/master/GoogleTestBootstrap.cs

First approach uses classic testing approach [TestClass] / [TestMethod] attributes, second uses C# scripting and besides that one it can also use .sln & .bat as input files, which is syncProj command line specific.

syncProj is full command line utility, and unlike Google unit testing - testing is by default not executed by application, it can be started separately using "-t" or "-tests", or from VS / GTA.

Build script: it appears that you have introduced a more robust way to figure out the location of VS, but why change the file name? Please apply the changes to the existing file such that the differences can be seen more easily.

Ok, I will. Does it makes any sense if I'll try to push one change at the time, verifying each step.

TestAdapterStarter: What's the purpose of this project? I have used it to debug test cases, it allow to intercept testhost.exe by using special environment variable - but it's only to try out how one or another thing works.

TestCaseDescriptor: Is there a particular reason for removing that class, or is this a refactoring for the sake of simplifying the code?

I have tried to add mechanism of being able to provide source code path / line position from test application itself - by default GTA queries it from .dll itself for C++. For C# those as provided by application itself, see

https://github.com/tapika/TestAdapterForGoogleTest/blob/master/GoogleTestAdapter/CSharpTestBootstrap/GoogleTestBootstrap.cs#L217

Btw - same mechanism could be applied for C++ as well (using FILE, LINE) , I suspect it could be much easier than current dia2.dll usage.

packages.config/app.config files: What are these changes for?

Not needed, git just showed a lot of changes, I've decided to commit them just to avoid confusion with my own changes.

Number of levels supported by VS test framework: I have no idea, I must admit :-)

I've noticed for C# scripting that normally I either need to have one folder and multiple .cs projects, or one test case is included in it's own folder, and in both cases shorter form would be to have directory first path step or script name itself without second path step. But in future it's possible to have unit test names as "subdir/test1.cs" - forward slash will not be used as a separator, but merely test itself, but as a quick walk-around I think it's acceptable.

tapika commented 5 years ago

image

I wanted also to get file extension visible and completely misusing now here "# GetParam() =" parameter.

I hope you as creator of GTA won't punish me for doing this. :)

tapika commented 5 years ago

Created video on my Google Test Adapter fork - maybe will help slightly better to understand what I'm driving in:

Application Driven Unit Testing https://www.youtube.com/watch?v=-miGEb7M3V8

csoltenborn commented 5 years ago

I had written the answer below this morning, but for some reason forgot to actually finish the comment. I have now very briefly looked at the linked presentation, and will have a look at the video probably at the weekend, but I thought that I'd share my initial thoughts in the meantime anyways... Stay tuned :-)

... Develop branch because that's where I do testing of the final version (before merging it to master and releasing)...

You seem to have put quite a bit of effort into your code, so I'm not sure how to say this politely: GTA is not going to support execution of C# tests. I have listed the main reasons for this above (and in fact, one suggestion would be to discuss whatever problems you have with the authors of the existing tooling for C# MS Test tests).

If I understand you correctly, then GTA happens to have a lot of the infrastructure you need for developing the kind of test adapter which solves your use cases, and because of that, you want to extend GTA with your use cases. Instead, what we could think about is to separate the gtest specific GTA code from the rest, and to try to build a "test adapter framework" which would allow others to implement their own adapter on top of it. However, that would be quite a bit of work...

Summarizing, I would suggest that if it comes to C# tests, you spend your efforts on a more appropriate project, e.g. this one.

tapika commented 5 years ago

GTA is not going to support execution of C# tests. I have listed the main reasons for this above (and in fact, one suggestion would be to discuss whatever problems you have with the authors of the existing tooling for C# MS Test tests).

That's is indeed possible, we could even raise bug report and wait couple of years for them to fix all issues and problems - but I have already custom GTA working, why to bother ?

Anyway, if not merged - I could create separate github project and develop further independently from GTA - that is indeed possible.

One of next problems which I need at the moment is to be able to determine call stack of native C++ + managed C# - .net framework supports managed side only, I have prototyped mixed mode call stack resolving once upon a time, https://sourceforge.net/projects/diagnostic/ it's doable but bit tricky.

testfx - quickly glanced, at least did not bite me (Another super heavy test framework, also non-deploy-able) , but maybe I have missed something, will look deeper into it later on. It did not exists on market place, wondering how early alpha it is.

As for C++ "scriptability", I'm planning to construct test framework on C++, similar to syncProj, but will try to change it's behavior slightly. Currently I'm offering text comparison at the end of one test, but I want to change so that comparison would happen in real-time, and DebugBreak() would be actually called if accepted data mismatches. (See C++ Reflection link, on last slide)

I still see potential in us cooperating, not making separate test adapters. :)

tapika commented 5 years ago

https://github.com/Microsoft/vstest-docs/blob/master/docs/diagnose.md#collect-traces-using-command-line

That is really funny, because we just today discussed about what we need to trace from our application (where I work on), and at the moment we use log4net on C# side, and on C++ we have custom #define's which can be enabled separately. It will be difficult to sell even to myself to use even more custom logging mechanisms besides our own.

I think main problem is that testing functionality closely relates to application being tested and testing framework should introduce as small overhead as possible.

csoltenborn commented 5 years ago

Sorry for not yet coming back to you - I hope that I find time around Eastern to have a thorough look at your presentation!

tapika commented 5 years ago

FYI, I have tested to change google test to make change similar to C# code on C++ side - see following pull request:

https://github.com/google/googletest/pull/2253

csoltenborn commented 4 years ago

I'm closing this PR since there hasn't been any activity for quite a while, and (to be honest) since I do not have time for discussing/working on these issues in the foreseeable future... Same holds for #277 .

If you want to keep working on this, please let me know anyways!

tapika commented 4 years ago

I would still prefer for you to integrate my changes - it would allow newest google test adapter to debug C# code out of box. At the moment however - newer C# unit test frameworks do sounds like a better alternative, as they can detect test list, as you edit the code.

I'm by myself focusing now on native C++, my idea is to recombine spdlog + google unit testing under same umbrella. Need to return back to C# still at some point of time, but there is still open issues, like going into .NET Core instead of .NET Framework.

csoltenborn commented 4 years ago

Sorry, but GTA is not going to support execution of C# tests for a number of reasons... Feel free however to fork GTA and run your own test adapter!

tapika commented 4 years ago

Can we chat more directly on subject ? either by mail or audio ? (tapika-at-yahoo-dot-com) Just want to get some understanding why C# cannot be just supported.

csoltenborn commented 4 years ago

Pretty busy at the moment - please remind me in a couple of weeks...

In the meantime here is some explanation: VsTest is not designed to have one test adapter covering all kinds of test frameworks, but for having several test adapters, each taking care of a single test framework. This is not a technical limitation, though, but makes a lot of sense:

If GTA contains code which would benefit other test adapters, then the opposite approach should be taken: pull out the necessary stuff from the GTA solution and make it accessible to other test adapters (instead of pulling the logic needed to run other test framework's tests into GTA).

tapika commented 4 years ago

Basically from C# testing perspective - the test application can be the same as in C++ case - that's standalone executable, which performs tests by itself.

In C++ case test "boostrap" is google test framework - that is https://github.com/google/googletest. (1) In C# case - I have coded my own test bootstrap https://github.com/tapika/TestAdapterForGoogleTest/tree/master/GoogleTestAdapter/TestCSharpClassic (2)

which pretty much resembles C++ bootstrap - it does not supports all command line parameter, but other command line parameters support could be added later on.

What is different from C++ versus C# perspective - pretty much nothing - except when attaching debugger you need to use slightly different mode to attach to executing process - see https://github.com/tapika/TestAdapterForGoogleTest/commit/3b66e13dc84a6aaa168e1b1ef2e7b6bd11c388a6. (*)

Basically maintaining my own testing framework (Tarmo's google test adapter) does not makes much sense, since my changes are rather little compared to what you have done with Google test adapter.

Even now - I would prefer just to take latest GTA which would work on vs2019, but I cannot - because it's missing debugger attach mechanism - see (*).

What I've realized afterwards - is that C# manages to catch only C# exceptions, and if you have application which consists from both - C++ and C#, then it's better to have native and managed mechanisms to catch exceptions.

This basically means that even if tested application and tested functionality is written in C#, exception handling mechanism must be based on C++, which in a turn suggest that instead of integrating (2), it would be preferable to integrate (1).

Of course C++ and C# have different mechanisms to handle exceptions, and they cannot be shared. See https://github.com/dotnet/diagnostics/issues/152.

And even call stack determination for C++ + C# mix application is non-trivial - see https://github.com/dotnet/runtime/issues/12405.

I do realize that ideally one application should not have two programming languages used in same project - because it introduces more complexities because of that.

But reality is that C# provides user interface and C++ provides fast performing core to us.

There is ongoing effort to standardize UI for C++ as well (C++ standard committee), maybe then we could reach the target of one programming language. But not before that one.

Btw, debug attach mode (*) is applicable to C++ components which use C++/clr - so if you want to debug it, then need also to switch debug mode.

If you are afraid of mixing up (1) & (2) bootstraps - maybe one approach could be to provide to GTA necessary meta data:

I still see that Google test framework as standalone self-testing executable is major step forward for C# unit testing, I have played around with other C# unit test frameworks, and you have other problems, like not being able to execute test from main thread.

But at the moment if you're not committed to supporting C#, there is nothing I can do about it, except using different test framework for C#.