winforms icon indicating copy to clipboard operation
winforms copied to clipboard

ATL ActiveX control with unit test

Open weltkante opened this issue 1 year ago • 4 comments

Contributes to #8294 and #8302

This is WIP and an early draft to get feedback. Open questions are:

  • where to put the controls? where to put the unit tests?
  • For the draft I put the control in the existing CMake "NativeTests" file and the tests along the other AxHost tests, however if that is the right place then the unit test project either needs the manifest reference added or an activation context needs to load the manifest dynamically.
  • is it ok to pull in the ATL headers into the NativeTests project? its awkward to not have a shared header (even if its not using the "precompiled header" feature), the way the individual headers pull in different platform headers in different orders (e.g. windows.h) means different cpp files see different configurations of the headers
  • currently only contains an ATL control, it might also be desireable to add an MFC control (which uses a different infrastructure and implementation on the C++ side but otherwise would be implemented similarly on the C# unit test side)

/cc @JeremyKuhne for comments and suggestions

Proposed changes

  • add ATL ActiveX control
    • test for control creation
    • test for event handling
  • [TODO] add MFC ActiveX control, if desired?
  • [TODO] variations of the tests using source generated interfaces instead of classic COM interop (might do that in a different PR)

Customer Impact

  • none, only increases test coverage

Regression?

  • No

Risk

  • none, only unit tests are changed

Before

  • lacking unit tests of ATL/MFC ActiveX controls

After

  • add unit tests for ATL/MFC ActiveX controls hosted via AxHost

Test methodology

  • automated tests

Accessibility testing

  • not included (accessibility is not customized on native side and retains default behavior)

Test environment(s)

  • 9.0.0-preview.2.24077.1 (whatever is pulled in by the build.cmd/start-vs.cmd on current main branch)
Microsoft Reviewers: Open in CodeFlow

weltkante avatar Jan 28 '24 19:01 weltkante

Codecov Report

Attention: 68 lines in your changes are missing coverage. Please review.

Comparison is base (2441a36) 73.16090% compared to head (1f6382e) 73.14391%.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #10754         +/-   ##
===================================================
- Coverage   73.16090%   73.14391%   -0.01699%     
===================================================
  Files           3087        3088          +1     
  Lines         633665      633737         +72     
  Branches       47402       47410          +8     
===================================================
- Hits          463595      463540         -55     
- Misses        166558      166685        +127     
  Partials        3512        3512                 
Flag Coverage Δ
Debug 73.14391% <5.55556%> (-0.01699%) :arrow_down:
integration 18.22953% <ø> (+0.00316%) :arrow_up:
production 46.60777% <ø> (-0.02063%) :arrow_down:
test 94.97593% <5.55556%> (-0.01852%) :arrow_down:
unit 43.51462% <ø> (-0.04544%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

codecov[bot] avatar Jan 28 '24 20:01 codecov[bot]

Sorry @weltkante - wrapped up in some other things. Hope to get a look in a few days.

JeremyKuhne avatar Feb 06 '24 00:02 JeremyKuhne

Sorry @weltkante - wrapped up in some other things. Hope to get a look in a few days.

No rush on this one, its not blocking anything, and in fact "just works" (once the remaining open points are decided), so its just about getting better coverage.

weltkante avatar Feb 06 '24 13:02 weltkante

Small update since the old code didn't build anymore inside VS after the recent .NET 9 preview was released, so I rebased onto the current main branch.

Seems the first draft wasn't working properly, VS was running x64 so the test was skipped and I mistakingly thought it worked, now it also works when running x86 tests and is actually executing the code.

While fixing this I noticed that the test project containing the AxHost tests is different from the test project containing the NativeTests.dll - so I tried adding a dependency to the CMake NativeTests but doesn't seem to work. If I copy the files (dll, tlb, manifest) manually it does work locally.

Maybe this particular AxHost test needs to move into the other project?

Anyways, no reason to priorize this, I just wanted to get it in working order before looking at the other ActiveX issues I have on my todo list ;-)

weltkante avatar Feb 17 '24 14:02 weltkante