[cDAC] Use Moq in CodeVersionsTest#110750
Conversation
|
Tagging subscribers to this area: @tommcdon |
There was a problem hiding this comment.
Are we sure we want to use Moq instead of NSubstitute? The former has had a history of poor security and potential spyware in more recent versions, while the latter is arguably easier to use, and is considered safe.
I would suggest really reconsidering the use of Moq in our codebase. If we really have to, then let's make sure we don't use a version newer than 4.18.4, which is the last one before these problems were encountered.
|
If we want to have a broader discussion on switching to a different mocking framework, I'm open. edit: It looks like we are pinned on version Line 202 in b33f755 |
That's good at least. We should be fine since we're sticking to the last not unsafe version, but I think moving away from it would be a good idea to bring up to discussion with others, as we can't update it anymore without putting our testing codebase at risk. |
|
There was a fair amount of discussion around this last year. We started with staying on an older version: #90222 (comment) I don't know where exactly we ended up more broadly, but I'd reach out to Rich if you want to find out or re-open the discussion. |
I think it's worth discussing it internally again at some point after the holidays because after reading that thread, I have to say I'm left with a bad taste in my mouth and even bigger concerns about using that library. Thanks for pointing the context out @elinor-fung! |
|
Is there a way to kill that |
|
Re Moq: I recommended Moq to Elinor because it's already referenced in the repo. Personally, I like FakeItEasy (which we also have already in dotnet-public and other repos use), but I didn't feel strongly enough about it to add the dependency. We should consider it as another option if we're reconsidering Moq usage. |
I just took a quick look at FakeItEasy and it seems like a great alternative option to consider. I agree, we should definitely suggest it next year when the Moq discussions are retaken. |
|
/azp run runtime |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/ba-g failure is dotnet/dnceng#3879 |
CodeVersionsTestused manually implemented "mocks" before we integrationMoq. Changes all tests to use theMoqframework in accordance with newer tests.Previous comments on draft pr: max-charlamb#2