Permit perf test duration to be set, seconds, by environment variable…#660
Conversation
… 'PERF_DURATION_SEC'
villegar
left a comment
There was a problem hiding this comment.
These changes look sensible to me! One question, may for the future: where does the function perf.reference.save write the outputs? I know we are currently ignoring these tests on CRAN, but they don't like you writing files to non-temp directories, so we might want to be careful.
timcadman
left a comment
There was a problem hiding this comment.
How about adding
ds.test_env$perf_duration_sec <- as.integer(Sys.getenv("PERF_DURATION_SEC", unset = "30"))
to setup.R and then calling in each performance file with ds.test_env$perf_duration_sec.
That way you wouldn't need to repeat that line in each performance test, which would be nice given that I'm planning to write performance tests for all the functions!
|
|
|
Some methods don't have a default of 30 seconds, e.g. on for connect-disconnect and void. |
Could we set 2-3 environment variables? Just trying to avoid a way of repeating the same line of code 100 times. Need to finish today but can have a think tomorrow (long train journey) |
|
We could have a method "perfDuration(unset)", but sometimes it feels clearer not to totally hide what is going on. As the test wouldn't be obviously linked to the value of "PERF_DURATION_SEC" environment variable. |
|
Yes I would be inclined to go for something like: Then in each test you can just do: Or |
… 'PERF_DURATION_SEC'
| # context("ds.abs::perf:0") | ||
| test_that("combine - performance", { | ||
| .durationSec <- 30 # seconds | ||
| .durationSec <- perf.testduration(30) |
There was a problem hiding this comment.
Given that you have set the default as 30, you could rely on the default, ieperf.testduration(). But maybe you've done it like this to make it explicit. I'll approve the PR, and leave it up to you if you want to change or not.
There was a problem hiding this comment.
My preference would be that a person reviewing the code would have an indication that the duration was alterable.
Can use environment variable 'PERF_DURATION_SEC', in seconds, to specify individual test's duration.