Permit perf test duration to be set, seconds, by environment variable…

[StuartWheater]

Can use environment variable 'PERF_DURATION_SEC', in seconds, to specify individual test's duration.

[villegar]

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]

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!

[StuartWheater]

perf.reference.save used to write to the specified perf profile file if a value for a profile was missing. But the CRAN didn't approve of change files in the repo, so it currently write it to a file, temporary directory.

[StuartWheater]

Some methods don't have a default of 30 seconds, e.g. on for connect-disconnect and void.

[timcadman]

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)

[StuartWheater]

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.

[timcadman]

Yes I would be inclined to go for something like:

 perf_duration <- function(default = 30) { base::as.integer(base::Sys.getenv("PERF_DURATION_SEC", unset = as.character(default)))
} 

Then in each test you can just do:
.durationSec <- perf_duration()

Or
.durationSec <- perf_duration(60)

[timcadman]

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.

[StuartWheater]

My preference would be that a person reviewing the code would have an indication that the duration was alterable.

[StuartWheater]

Can use environment variable 'PERF_DURATION_SEC', in seconds, to specify individual test's duration.

[villegar]

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]

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!

[StuartWheater]

perf.reference.save used to write to the specified perf profile file if a value for a profile was missing. But the CRAN didn't approve of change files in the repo, so it currently write it to a file, temporary directory.

[StuartWheater]

Some methods don't have a default of 30 seconds, e.g. on for connect-disconnect and void.

[timcadman]

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)

[StuartWheater]

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.

[timcadman]

Yes I would be inclined to go for something like:

 perf_duration <- function(default = 30) { base::as.integer(base::Sys.getenv("PERF_DURATION_SEC", unset = as.character(default)))
} 

Then in each test you can just do:
.durationSec <- perf_duration()

Or
.durationSec <- perf_duration(60)

[timcadman]

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.

[StuartWheater]

My preference would be that a person reviewing the code would have an indication that the duration was alterable.

[StuartWheater]

Can use environment variable 'PERF_DURATION_SEC', in seconds, to specify individual test's duration.

[villegar]

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]

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!

[StuartWheater]

perf.reference.save used to write to the specified perf profile file if a value for a profile was missing. But the CRAN didn't approve of change files in the repo, so it currently write it to a file, temporary directory.

[StuartWheater]

Some methods don't have a default of 30 seconds, e.g. on for connect-disconnect and void.

[timcadman]

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)

[StuartWheater]

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.

[timcadman]

Yes I would be inclined to go for something like:

 perf_duration <- function(default = 30) { base::as.integer(base::Sys.getenv("PERF_DURATION_SEC", unset = as.character(default)))
} 

Then in each test you can just do:
.durationSec <- perf_duration()

Or
.durationSec <- perf_duration(60)

[timcadman]

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.

[StuartWheater]

My preference would be that a person reviewing the code would have an indication that the duration was alterable.

[StuartWheater]

Can use environment variable 'PERF_DURATION_SEC', in seconds, to specify individual test's duration.

[villegar]

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]

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!

[StuartWheater]

perf.reference.save used to write to the specified perf profile file if a value for a profile was missing. But the CRAN didn't approve of change files in the repo, so it currently write it to a file, temporary directory.

[StuartWheater]

Some methods don't have a default of 30 seconds, e.g. on for connect-disconnect and void.

[timcadman]

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)

[StuartWheater]

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.

[timcadman]

Yes I would be inclined to go for something like:

 perf_duration <- function(default = 30) { base::as.integer(base::Sys.getenv("PERF_DURATION_SEC", unset = as.character(default)))
} 

Then in each test you can just do:
.durationSec <- perf_duration()

Or
.durationSec <- perf_duration(60)

[timcadman]

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.

[StuartWheater]

My preference would be that a person reviewing the code would have an indication that the duration was alterable.

[StuartWheater]

Can use environment variable 'PERF_DURATION_SEC', in seconds, to specify individual test's duration.

[villegar]

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]

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!

[StuartWheater]

perf.reference.save used to write to the specified perf profile file if a value for a profile was missing. But the CRAN didn't approve of change files in the repo, so it currently write it to a file, temporary directory.

[StuartWheater]

Some methods don't have a default of 30 seconds, e.g. on for connect-disconnect and void.

[timcadman]

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)

[StuartWheater]

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.

[timcadman]

Yes I would be inclined to go for something like:

 perf_duration <- function(default = 30) { base::as.integer(base::Sys.getenv("PERF_DURATION_SEC", unset = as.character(default)))
} 

Then in each test you can just do:
.durationSec <- perf_duration()

Or
.durationSec <- perf_duration(60)

[timcadman]

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.

[StuartWheater]

My preference would be that a person reviewing the code would have an indication that the duration was alterable.

[StuartWheater]

Can use environment variable 'PERF_DURATION_SEC', in seconds, to specify individual test's duration.

[villegar]

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]

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!

[StuartWheater]

perf.reference.save used to write to the specified perf profile file if a value for a profile was missing. But the CRAN didn't approve of change files in the repo, so it currently write it to a file, temporary directory.

[StuartWheater]

Some methods don't have a default of 30 seconds, e.g. on for connect-disconnect and void.

[timcadman]

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)

[StuartWheater]

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.

[timcadman]

Yes I would be inclined to go for something like:

 perf_duration <- function(default = 30) { base::as.integer(base::Sys.getenv("PERF_DURATION_SEC", unset = as.character(default)))
} 

Then in each test you can just do:
.durationSec <- perf_duration()

Or
.durationSec <- perf_duration(60)

[timcadman]

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.

[StuartWheater]

My preference would be that a person reviewing the code would have an indication that the duration was alterable.

[StuartWheater]

Can use environment variable 'PERF_DURATION_SEC', in seconds, to specify individual test's duration.

[villegar]

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]

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!

[StuartWheater]

perf.reference.save used to write to the specified perf profile file if a value for a profile was missing. But the CRAN didn't approve of change files in the repo, so it currently write it to a file, temporary directory.

[StuartWheater]

Some methods don't have a default of 30 seconds, e.g. on for connect-disconnect and void.

[timcadman]

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)

[StuartWheater]

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.

[timcadman]

Yes I would be inclined to go for something like:

 perf_duration <- function(default = 30) { base::as.integer(base::Sys.getenv("PERF_DURATION_SEC", unset = as.character(default)))
} 

Then in each test you can just do:
.durationSec <- perf_duration()

Or
.durationSec <- perf_duration(60)

[timcadman]

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.

[StuartWheater]

My preference would be that a person reviewing the code would have an indication that the duration was alterable.

[StuartWheater]

Can use environment variable 'PERF_DURATION_SEC', in seconds, to specify individual test's duration.

[villegar]

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]

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!

[StuartWheater]

perf.reference.save used to write to the specified perf profile file if a value for a profile was missing. But the CRAN didn't approve of change files in the repo, so it currently write it to a file, temporary directory.

[StuartWheater]

Some methods don't have a default of 30 seconds, e.g. on for connect-disconnect and void.

[timcadman]

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)

[StuartWheater]

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.

[timcadman]

Yes I would be inclined to go for something like:

 perf_duration <- function(default = 30) { base::as.integer(base::Sys.getenv("PERF_DURATION_SEC", unset = as.character(default)))
} 

Then in each test you can just do:
.durationSec <- perf_duration()

Or
.durationSec <- perf_duration(60)

[timcadman]

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.

[StuartWheater]

My preference would be that a person reviewing the code would have an indication that the duration was alterable.