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

Remove system.random #6997

Open
wants to merge 2 commits into
base: dev
from

Conversation

Projects
None yet
3 participants
@cristinamanum
Copy link
Contributor

cristinamanum commented Mar 14, 2019

Remove system.random usage.

@cristinamanum cristinamanum requested review from zhhyu , xavierdecoster , skofman1 , shishirx34 , scottbommarito , loic-sharma , ryuyu , agr and joelverhagen and removed request for skofman1 Mar 14, 2019

using Moq;
using NuGet.Services.Entities;
using Xunit;
using NuGetGallery.TestUtils;

This comment has been minimized.

@joelverhagen

joelverhagen Mar 14, 2019

Member

sort usings


namespace NuGetGallery.Infrastructure
{
public class Base32EncoderTests
{
private Random _random = new Random(0);
private SecureRandomNumberGenerator _random = new SecureRandomNumberGenerator();

This comment has been minimized.

@joelverhagen

joelverhagen Mar 14, 2019

Member

Is this actually a requirement for test code?

This comment has been minimized.

@scottbommarito

scottbommarito Mar 14, 2019

Member

I agree, using secure random numbers in test code seems very arbitrary...

@@ -6,12 +6,13 @@
using System.Text;
using NuGetGallery.Infrastructure.Authentication;
using Xunit;
using NuGetGallery.TestUtils;

This comment has been minimized.

@joelverhagen

joelverhagen Mar 14, 2019

Member

Sort usings

/// A number generator using RNGCryptoServiceProvider.
/// Based on http://csharphelper.com/blog/2014/08/use-a-cryptographic-random-number-generator-in-c/
/// </summary>
public class SecureRandomNumberGenerator

This comment has been minimized.

@joelverhagen

joelverhagen Mar 14, 2019

Member

This should be in a shared place if this really a new requirement for us.

This comment has been minimized.

@joelverhagen

joelverhagen Mar 14, 2019

Member

(ServerCommon)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.