microsoft / wil

Windows Implementation Library
MIT License
2.57k stars 234 forks source link

Consume Detours for function mocking #421

Closed dunhor closed 8 months ago

dunhor commented 8 months ago

A little background - internal to the Windows repo there's a library for mocking functions for the purpose of writing tests. As of this writing, this library has not yet been made public. The original tests for WIL (before being made open source) relied on this library for a number of things. When porting the tests to this repo, the tests that rely on this library either needed to be modified to use alternative means for testing the same functionality or omitted entirely. This led to a number of hacks to try and achieve the same functionality, some of which have historically caused issues.

This change is an attempt to replace the use of that library with something that uses the Detours library to achieve something similar (it's noteworthy that the internal library also uses Detours). This code isn't intended to be as feature rich as the internal library and is targeted at providing just what's necessary for our tests. In particular, there are two types used for mocking:

Both of these types are designed to be able to be constructed with a lambda since testing function pointers + globals is often a mess. Additionally, both are designed such that calls are non-reentrant on the thread that called them. This is nice because it allows the detour to just invoke the function it's detouring by name as opposed to needing to do something like reference the object holding it, however this also means that some scenarios may not work 100% as expected (e.g. your detour => detoured function => other function... => detoured function).

I've updated relevant tests to use these types as opposed to the work arounds we've been using & added some tests that were never copied in the first place. Now, this change isn't without some negatives:

  1. In order to use Detours, the tests now have a dependency on vcpkg. This is an extra setup step - albeit one most people will only need to do once - however it does increase the "barrier to entry" slightly. Aside - I also took the opportunity to switch to consuming Catch2 via vcpkg while I was at it. NOTE: I'm unsure if this may have consequences for code consuming WIL from vcpkg.
  2. I bumped the minimum C++ standard version that the tests build against to C++17 because it was a huge pain to write the detoured function types without inline static variables. I don't think this will be a major issue - C++17 has been out for quite some time now and the Windows build switched to C++17 a while ago, however it's noteworthy that we no longer have build coverage.

Ultimately, these tradeoffs seem worth it to me; just calling them out to see what others think.

Fixes #393

dunhor commented 8 months ago

Note that an alternative to adding a dependency on vcpkg is to instead check in & build Detours ourselves. Personally, I'm not a huge fan, but...

asklar commented 8 months ago

hooray :)

dunhor commented 8 months ago

[celebrate] Duncan Horn reacted to your message:


From: Alexander Sklar @.> Sent: Friday, January 5, 2024 4:56:13 AM To: microsoft/wil @.> Cc: Duncan Horn @.>; State change @.> Subject: Re: [microsoft/wil] Consume Detours for function mocking (PR #421)

hooray :)

— Reply to this email directly, view it on GitHubhttps://github.com/microsoft/wil/pull/421#issuecomment-1878124219, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AJROQIDMQJY5VLSTWSPFEQTYM6BW3AVCNFSM6AAAAABBKUEVG6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZYGEZDIMRRHE. You are receiving this because you modified the open/close state.Message ID: @.***>