Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ATL ActiveX control with unit test #10754

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Conversation

weltkante
Copy link
Contributor

@weltkante weltkante commented Jan 28, 2024

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

@ghost ghost assigned weltkante Jan 28, 2024
@ghost ghost added the draft draft PR label Jan 28, 2024
Copy link

codecov bot commented Jan 28, 2024

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%) ⬇️
integration 18.22953% <ø> (+0.00316%) ⬆️
production 46.60777% <ø> (-0.02063%) ⬇️
test 94.97593% <5.55556%> (-0.01852%) ⬇️
unit 43.51462% <ø> (-0.04544%) ⬇️

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

@JeremyKuhne
Copy link
Member

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

@weltkante
Copy link
Contributor Author

weltkante commented Feb 6, 2024

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
Copy link
Contributor Author

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 ;-)

@JeremyKuhne JeremyKuhne added this to the .NET 10.0 milestone Jul 11, 2024
@lonitra lonitra changed the base branch from main to feature/10.0 July 23, 2024 00:59
@lonitra lonitra changed the base branch from feature/10.0 to main August 15, 2024 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
draft draft PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants