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

Add spec to test correct random distribution #1450

Open
mgreter opened this issue Jul 30, 2019 · 4 comments
Open

Add spec to test correct random distribution #1450

mgreter opened this issue Jul 30, 2019 · 4 comments

Comments

@mgreter
Copy link
Contributor

mgreter commented Jul 30, 2019

Not sure we have such a spec test yet, if so please ignore.

$lower: 0;
$upper: 0;

@for $i from 1 through 5000 {
  $random: random(2);
  @if ($random == 1) {
    $lower: $lower + 1;
  }
  @else {
    $upper: $upper + 1;
  }
}

result {
  lower: if($lower > 4500, "OK", "Suspicious");
  upper: if($upper > 4500, "OK", "Suspicious");
}

$lower: 0;
$upper: 0;

@for $i from 1 through 5000 {
  $random: random(null);
  @if ($random < 0.5) {
    $lower: $lower + 1;
  }
  @else {
    $upper: $upper + 1;
  }
}

result {
  lower: if($lower > 4500, "OK", "Suspicious");
  upper: if($upper > 4500, "OK", "Suspicious");
}
@nex3
Copy link
Contributor

nex3 commented Aug 12, 2019

I'm not sure I'm a fan of adding a spec that only passes probabilistically. I feel like it's probably fairly easy to manually verify that a random() function actually produces different numbers in a fairly random way, and users aren't going to be using this for anything security-related so I don't think there's a pressing need to verify it with a high degree of certainty.

@mgreter
Copy link
Contributor Author

mgreter commented Aug 13, 2019

I agree that this is not a test about anything security related, but I had made an error about random(2) that was not catched by any spec test, and I'm pretty sure we already have similar probabilistic test for random already.

@mgreter
Copy link
Contributor Author

mgreter commented Aug 13, 2019

https://github.com/sass/sass-spec/blob/89531ae1c307bba140bf3d853599afa2ac55dc35/spec/core_functions/math/random.hrx ... the intention of the test is not to assure random produces a random number, but also in the expected "from" "to" range ...

@nex3
Copy link
Contributor

nex3 commented Aug 14, 2019

The existing random() tests are only probabilistic if the random() function is behaving incorrectly. If it's behaving correctly, they're guaranteed to pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants