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

Reconsider using Moq in testing #90222

Closed
GerardSmit opened this issue Aug 9, 2023 · 27 comments
Closed

Reconsider using Moq in testing #90222

GerardSmit opened this issue Aug 9, 2023 · 27 comments

Comments

@GerardSmit
Copy link

GerardSmit commented Aug 9, 2023

I'm not sure if .NET is aware of the current situation with Moq, but they've added a analyzer in version 4.20 that asks the developer in the IDE through warnings to support the project through an external closed-source library "SponsorLink".

This is done by checking if you're a sponsor of the GitHub maintainer by getting the hash of the developer, and checking through an infrastructure that is unknown by the public. If you're not an supporter, the first time the build is delayed by 1 second, then by 100ms (so you read the message).

This means the analyzer does the following:

  1. Get the e-mail from your computer
  2. Hash the e-mail and send it to an unknown server
    Note: As noted by other people this could also be an GDPR issue...
  3. Write and read files to check if you already seen the message
    Note: This had a bug in Mac/Linux (see https://github.com/moq/moq/issues/1369) which caused the build to fail...
  4. Add an warning to the error list and delay the build
    Note: If WarnAsError is enabled, you'll get an error instead.

An .NET/Roslyn maintainer (@sharwell) already reached out 4 months ago to the developer that this should not be done through analyzers: devlooped/SponsorLink#10, but this was ignored.

Moq is used by the testing infrastructure (this can be checked here: https://github.com/search?q=repo%3Adotnet%2Fruntime+language%3AXML+Moq&type=code). The used version, 4.18.4, does not include SponsorLink. Version 4.20 or higher is affected.

For more information, see the active issue: https://github.com/moq/moq/issues/1372

An example of SponsorLink blocking the build:

image

More information about SponsorLink can be found here: https://github.com/devlooped/SponsorLink/

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 9, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 9, 2023
@Atulin
Copy link

Atulin commented Aug 9, 2023

Crazy that it weaseled its way even into the very .NET Runtime repo

@vcsjones vcsjones added area-Meta and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 9, 2023
@ghost
Copy link

ghost commented Aug 9, 2023

Tagging subscribers to this area: @dotnet/area-meta
See info in area-owners.md if you want to be subscribed.

Issue Details

I'm not sure if .NET is aware of the current situation with Moq, but they've added a analyzer in version 4.20 that asks the developer in the IDE through warnings to support the project through an external closed-source library "SponsorLink".

This is done by checking if you're a sponsor of the GitHub maintainer by getting the hash of the developer, and checking through an infrastructure that is unknown by the public. If you're not an supporter, the first time the build is delayed by 1 second, then by 100ms (so you read the message).

This means the analyzer does the following:

  1. Get the e-mail from your computer
  2. Hash the e-mail and send it to an unknown server
    Note: As noted by other people this could also be an GDPR issue...
  3. Write and read files to check if you already seen the message
    Note: This had a bug in Mac/Linux (see https://github.com/moq/moq/issues/1369) which caused the build to fail...
  4. Add an warning to the error list and delay the build
    Note: If WarnAsError is enabled, you'll get an error instead.

An .NET/Roslyn maintainer (@sharwell) already reached out 4 months ago to the developer that this should not be done through analyzers: devlooped/SponsorLink#10, but this was ignored.

Moq is used by the testing infrastructure (this can be checked here: https://github.com/search?q=repo%3Adotnet%2Fruntime+language%3AXML+Moq&type=code). The used version, 4.18.4, does not include SponsorLink. Version 4.20 or higher is affected.

For more information, see the active issue: https://github.com/moq/moq/issues/1372

An example of SponsorLink blocking the build:

image

More information about SponsorLink can be found here: https://github.com/devlooped/SponsorLink/

Author: GerardSmit
Assignees: -
Labels:

area-Meta, untriaged

Milestone: -

@Joe4evr
Copy link
Contributor

Joe4evr commented Aug 9, 2023

4. Add an warning to the error list and delay the build
Note: If WarnAsError is enabled, you'll get an error instead.

Additionally, According to Nick Craver, your build will be broken by WarnAsError even if you're a sponsor.

@MichaelKetting
Copy link
Contributor

There is a follow-up discussion here: https://github.com/moq/moq/issues/1374

Apperently, the change is going to get reverted in v4.20.2. What it means for longterm viability of Moq, I cannot say. Am lookng forward to seeing how the dotnet team is going to handle this.

For background information:
For me, I'm considering a fork of Moq in the event that the project is no longer considered trustworthy since we only recently completed a migration from RhinoMocks to Moq and I am not interested in spending another several hundred hours doing another migration. Originally, I chose this Moq library because it's a) used by Microsoft and b) has a syntax more compatible with what Rhino.Mocks offered as compared to FakeIt.Easy.

@samtrion
Copy link

samtrion commented Aug 9, 2023

Apperently, the change is going to get reverted in v4.20.2. What it means for longterm viability of Moq, I cannot say. Am lookng forward to seeing how the dotnet team is going to handle this.

The reverted version 4.20.2 is unfortunately only cosmetic. The reference and functionality remains in MOQ.

@kzu
Copy link
Contributor

kzu commented Aug 9, 2023

@samtrion that is false. The reference is gone, and the analyzer is no longer packed with 4.20.2.

Points 1 and 2 will be addressed when it's reintroduced: will most likely just use a GUID.

@RonSijm
Copy link

RonSijm commented Aug 9, 2023

Apperently, the change is going to get reverted in v4.20.2. What it means for longterm viability of Moq, I cannot say. Am lookng forward to seeing how the dotnet team is going to handle this.

They removed the package reference due to compatibility changes with Mac and Linux - not because of community feedback

@samtrion that is false. The reference is gone, and the analyzer is no longer packed with 4.20.2.

Points 1 and 2 will be addressed when it's reintroduced: will most likely just use a GUID.

...

reintroduced

Doesn't sound like they intent do remove it entirely ಠ_ಠ

@karl-sjogren
Copy link

karl-sjogren commented Aug 9, 2023

Points 1 and 2 will be addressed when it's reintroduced: will most likely just use a GUID.

...

reintroduced

Doesn't sound like they intent do remove it entirely ಠ_ಠ

It also doesn't say anything about the point about the intentional slow downs of the builds for non sponsoring users. If I want to contribute to the dotnet runtime I shouldn't be forced to sponsor Moq just to avoid keeping my builds going full speed..

@samtrion
Copy link

samtrion commented Aug 9, 2023

Apperently, the change is going to get reverted in v4.20.2. What it means for longterm viability of Moq, I cannot say. Am lookng forward to seeing how the dotnet team is going to handle this.

They removed the package reference due to compatibility changes with Mac and Linux - not because of community feedback

@samtrion that is false. The reference is gone, and the analyzer is no longer packed with 4.20.2.

Points 1 and 2 will be addressed when it's reintroduced: will most likely just use a GUID.

...

reintroduced

Doesn't sound like they intent do remove it entirely ಠ_ಠ

Yes, unfortunately, there is no insight of the misconduct and further efforts to enforce this. Here, it is the community and certainly one or the other security officer who decides.

@RassK
Copy link

RassK commented Aug 9, 2023

If I want to contribute to the dotnet runtime I shouldn't be forced to sponsor Moq just to avoid keeping my builds going full speed..

It's just not about Moq, but think if you are using tens of OSS libraries using SponsorLinker. Some of them with nested SponsorLinker (git submodules). So at the end you have to be subscribed everywhere to even build something.
There are major OSS software frameworks (ecoms, cms, etc) built on top of other OSS metapackages, even a single plugin can affect the build.

So in the end your console will be bloated with "thank you"s or "Sorry, sponsor me!" messages? then a massive delay follows.
Remote server goes down - again delay + delay for not supporting?

I see the OSS financing issue but unfortunately this solution is born dead.

@kzu
Copy link
Contributor

kzu commented Aug 9, 2023

Remote server == Azure CDN > Azure Blob Storage. If that goes down, there will be far serious issues :)

Happy to receive feedback which I was hoping to collect when I [announced}(https://www.cazzulino.com/sponsorlink.html) and started mentioning (and shipping!) SponsorLink on smaller packages. Better late than never, I suppose :)

I'm happy to explore other solutions that aren't "born dead" if they can be worked out and actually produce the intended results. Sure the "thank you" can be elimitated. How would you nag "properly" for people to sponsor the projects they use?

And let's address the elephant in the room: Microsoft has been leveraging and enjoying Moq everywhere and in almost every project, and yet it NEVER approached me for support. Neither any devs in any teams (except for @KirillOsenkov from way back then, right when I created my sponsor account). Mind you, @aws DID!

Does anyone think THAT's ok?

@LarsWesselius
Copy link

LarsWesselius commented Aug 9, 2023

And let's address the elephant in the room: Microsoft has been leveraging and enjoying Moq everywhere and in almost every project, and yet it NEVER approached me for support. Neither any devs in any teams (except for @KirillOsenkov from way back then, right when I created my sponsor account). Mind you, @aws DID!

Does anyone think THAT's ok?

Yeah cause you decided to make an open source project with a license that permits this usage. If you want to change it, you can but not by introducing nagware/spyware and violating GDPR in the process

@kzu
Copy link
Contributor

kzu commented Aug 9, 2023

Hey @LarsWesselius here too! I see your're very GDPR-concerned. I wonder if that's gone and the analyzer were OSS too, what would the "concern" be🤔?

@karl-sjogren
Copy link

If I want to contribute to the dotnet runtime I shouldn't be forced to sponsor Moq just to avoid keeping my builds going full speed..

It's just not about Moq, but think if you are using tens of OSS libraries using SponsorLinker. Some of them with nested SponsorLinker (git submodules). So at the end you have to be subscribed everywhere to even build something. There are major OSS software frameworks (ecoms, cms, etc) built on top of other OSS metapackages, even a single plugin can affect the build.

So in the end your console will be bloated with "thank you"s or "Sorry, sponsor me!" messages? then a massive delay follows. Remote server goes down - again delay + delay for not supporting?

I completely agree, but after this I'm not particularly worried that anyone else will adopt this approach 😛 (also, if you want to use SponsorLink in your own project, you must sponsor SponsorLink, so yeah..)

@ColinM9991
Copy link

How would you nag "properly" for people to sponsor the projects they use?

@kzu Simple, do what Identity Server did. Deprecate support for the previous versions and move to a paid version. Make that clear by archiving the previous version and making a new repository and new NuGet package. Based on your ideal outcome, and how you describe the problem, then surely you're not interested in simply pushing an alert on users since they can just work around that.

You keep gatekeeping the feedback on the basis that hundreds of people aren't contributing to massive OSS projects, yet many big names across StackExchange/Stackoverflow, Microsoft and more have spoken out against the way in which you've done this, but you still defend it rather than accepting you've had a lapse in judgement.

@RassK
Copy link

RassK commented Aug 9, 2023

I'm happy to explore other solutions that aren't "born dead" if they can be worked out and actually produce the intended results. Sure the "thank you" can be elimitated. How would you nag "properly" for people to sponsor the projects they use?

As @LarsWesselius pointed if it's OSS and license is permitting then you should not "nag" people. You could nag people if they violate the license. You could have changed the license for corporate users or what ever the red border should be, but this really means building a proper business.

And let's address the elephant in the room: Microsoft has been leveraging and enjoying Moq everywhere and in almost every project, and yet it NEVER approached me for support. Neither any devs in any teams (except for @KirillOsenkov from way back then, right when I created my sponsor account). Mind you, @aws DID!

Lots of developers have (because the license allows to do it). I guess I have used it almost 10 years in most of the projects.
Being a random engineer in a large project, I do not think I should be collecting "donations" for OSS projects. If you want to address business people, it should be done adequately. Currently you are targeting developers but the actual target should be the final beneficiary.

In final - own the copyrights, change the license, charge accordingly. Every non pirate would have been fine and either forwards the bill to business department or makes changes accordingly.

I have read that you do not want to run business. In any case you could have found a vendor to front your business.

@dylan-asos
Copy link

Simple, do what Identity Server did. Deprecate support for the previous versions and move to a paid version. Make that clear by archiving the previous version and making a new repository and new NuGet package

Absolutely this - it was a great example of how to change the model & communicate it clearly.

@Pilchard123
Copy link

Though there was, uh, "quite a kerfuffle" about the Duende change @dylan-asos . It did die down eventually, but even with a change like that there would be some pain.

@cmjdiff
Copy link

cmjdiff commented Aug 9, 2023

Hey @LarsWesselius here too! I see your're very GDPR-concerned. I wonder if that's gone and the analyzer were OSS too, what would the "concern" be🤔?

@kzu At this point, the "concern" has moved on to your inability to take and learn from feedback. The response has been very clear: The concept of SponsorLink is unambiguously bad. The correct response is to sincerely apologise, remove it from your projects, make it unavailable, and commit to never pulling anything like this again. Your response has been to pull it for technical reasons, tell people in one thread that it's gone and in another thread that you're going to reintroduce it, and then get mad at the people calling out your misconduct.

That is why people all over the world are removing Moq from their projects today. That is why teams are incurring costs and burning effort on this rather than sending the equivalent cost to you in sponsorship. They're not doing it because they don't want to pay. They're not doing it to spite you. They're doing it because you did something likely illegal, that would open them up to liability for the same, and when challenged on it you didn't listen.

@kzu
Copy link
Contributor

kzu commented Aug 9, 2023

I didn't listen? I removed the privacy concern within hours (had to wake up first!) and assured everyone the privacy side (email hashing) won't come back, ever. At this point I question whether that was the true concern. That's my point.

I'm not going to apologize for making it obvious that the current state of OSS sustainability is quite bad. And I'm not going to stop trying to find a solution.

@jkdmyrs
Copy link

jkdmyrs commented Aug 9, 2023

@kzu

And let's address the elephant in the room: Microsoft has been leveraging and enjoying Moq everywhere and in almost every project, and yet it NEVER approached me for support. Neither any devs in any teams (except for @KirillOsenkov from way back then, right when I created my sponsor account). Mind you, @aws DID!

Does anyone think THAT's ok?

Yes? Microsoft isn't indebted to you because they use your package. That's the whole idea behind FOSS... you can just use it.

I echo what others are saying; it sounds like you need to re-license the product such that you need to pay to use it, because that's clearly what you want. Goodluck.

@cmjdiff
Copy link

cmjdiff commented Aug 9, 2023

I didn't listen? I removed the privacy concern within hours (had to wake up first!) and assured everyone the privacy side (email hashing) won't come back, ever.

You didn't "remove the privacy concern". You rolled back a change because of a technical issue.
You didn't "assure everyone [it] won't come back, ever". You did the exact opposite, by leaving the enabling code there and talking about "when" it comes back.
You've made it clear that you see no problem with what you've done. You're acting like everything's okay now you put the cookie back in the jar.
What you're saying doesn't match up with what you're doing, and given the choice most people are going to believe your actions over your words.

I'm not going to apologize for making it obvious that the current state of OSS sustainability is quite bad.

See, this is precisely what I mean about not listening. Nobody's asking you to apologise for the state of OSS. They're asking you to apologise for being an asshole, and to not be an asshole in future.

@richlander
Copy link
Member

richlander commented Aug 9, 2023

We use this library, as the OP notes. We don't believe that we've moved to the new version that has the problematic behavior. If we find we have, we'll revert those changes immediately.

We plan to wait and see what the situation looks like after the dust settles. We see no reason to rush given we have no reason to believe that there is a problem with the earlier versions. We'll come up with a remediation plan when we have more information to act on. We would like to see the changed reverted. If that doesn't happen, then we'll need adopt an alternative.

To be clear, this new behavior is deeply problematic (for multiple reasons) and unacceptable for a dependency of ours.

I'm closing this issue since we're in receipt of the "reconsider" statement and well into our own conversations on the topic. This repo isn't the right place to discuss the general topic. The Moq repo or social media are better places.

Thanks for raising awareness @GerardSmit. That's appreciated. Please always raise topics like this since it is indeed possible that we're unaware or don't have all the information.

@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Aug 9, 2023
@kzu
Copy link
Contributor

kzu commented Aug 10, 2023

We would like to see the changed reverted. I

It already has. The dependency is no longer shipping in latest Moq.

@GurliGebis
Copy link
Contributor

We would like to see the changed reverted. I

It already has. The dependency is no longer shipping in latest Moq.

But you plan on reintroducing it, so they should keep it locked at the current version or remove it.
Removing it from the newest version, waiting for things to cool down, and then reintroducing it....
Abusing analysers to read files and send it off to your own server is malware behaviour, plain and simple, no matter the intentions.

@madhon
Copy link

madhon commented Aug 10, 2023

We would like to see the changed reverted. I

It already has. The dependency is no longer shipping in latest Moq.

As already mentioned, you only reverted it due to it breaking MacOS/Linux builds, and you still intend bringing it back in some form.

@cartermp
Copy link

Recommend locking this thread because it'll just veer off into unrelated territory

@dotnet dotnet locked as off-topic and limited conversation to collaborators Aug 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests