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

Test is flaky #308

Closed
MichaelChirico opened this issue Oct 9, 2023 · 4 comments
Closed

Test is flaky #308

MichaelChirico opened this issue Oct 9, 2023 · 4 comments

Comments

@MichaelChirico
Copy link
Contributor

MichaelChirico commented Oct 9, 2023

This test can fail randomly:

expect_equal(
sort(unique(value_sample(value_seq, 40))),
value_seq$values
)

library(dials)
library(testthat)

value_seq <- new_quant_param(
  type = "double",
  range = c(0.0, 1.0),
  inclusive = c(TRUE, TRUE),
  trans = NULL,
  values = (0:5) / 5,
  label = c(param = "param")
)
test_fails <- function(n = 40L) {
  value_sample(value_seq, n) |>
  unique() |>
  sort() |>
  expect_equal(value_seq$values) |>
  tryCatch(error = identity) |>
  inherits("error")
}
table(replicate(1000, test_fails()))
# FALSE  TRUE 
#   996     4 
@MichaelChirico
Copy link
Contributor Author

MichaelChirico commented Oct 9, 2023

If I up the sample size to 50L it is less than 0.1% failure:

table(replicate(5000, test_fails(50)))
# FALSE  TRUE 
#  4999     1

@hfrick
Copy link
Member

hfrick commented Oct 10, 2023

Thanks for the issue and PR @MichaelChirico ! You may have been looking at an outdated version of this test though, that's not what it looks like on main now. I've changed it a while back in #297. I appreciate the contribution but I'm okay with the current version and am going to close the issue and PR.

@hfrick hfrick closed this as completed Oct 10, 2023
@MichaelChirico
Copy link
Contributor Author

not sure how I missed the updated test! I agree your implementation is much preferable since it has probability 1 of succeeding. thanks!

@github-actions
Copy link

This issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.

@github-actions github-actions bot locked and limited conversation to collaborators Oct 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants