You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/04/20 18:59:03 UTC
[GitHub] [arrow] assignUser opened a new pull request, #12940: ARROW-15015: [R] Test / CI flag for ensuring all tests are run?
assignUser opened a new pull request, #12940:
URL: https://github.com/apache/arrow/pull/12940
@jonkeane there are still a bunch of skips but the once relying on the "helpe-skip" functions should all be forced to run now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] assignUser commented on pull request #12940: ARROW-15015: [R] Test / CI flag for ensuring all tests are run?
Posted by GitBox <gi...@apache.org>.
assignUser commented on PR #12940:
URL: https://github.com/apache/arrow/pull/12940#issuecomment-1105463307
Should be merge after https://github.com/apache/arrow/pull/12946
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] assignUser commented on a diff in pull request #12940: ARROW-15015: [R] Test / CI flag for ensuring all tests are run?
Posted by GitBox <gi...@apache.org>.
assignUser commented on code in PR #12940:
URL: https://github.com/apache/arrow/pull/12940#discussion_r855406310
##########
r/tests/testthat/helper-skip.R:
##########
@@ -69,12 +76,14 @@ skip_on_linux_devel <- function() {
}
skip_if_r_version <- function(r_version) {
+ if (force_tests) return()
if (getRversion() <= r_version) {
skip(paste("R version:", getRversion()))
}
}
process_is_running <- function(x) {
+ if (force_tests) return(TRUE)
Review Comment:
Yeah makes sense, as it caught me too :D
##########
r/tests/testthat/helper-skip.R:
##########
@@ -69,12 +76,14 @@ skip_on_linux_devel <- function() {
}
skip_if_r_version <- function(r_version) {
+ if (force_tests) return()
if (getRversion() <= r_version) {
skip(paste("R version:", getRversion()))
}
}
process_is_running <- function(x) {
+ if (force_tests) return(TRUE)
Review Comment:
Yeah makes sense, as it caught me too :D (the inverted behavior)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #12940: ARROW-15015: [R] Test / CI flag for ensuring all tests are run?
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #12940:
URL: https://github.com/apache/arrow/pull/12940#issuecomment-1104381990
https://issues.apache.org/jira/browse/ARROW-15015
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] assignUser commented on a diff in pull request #12940: ARROW-15015: [R] Test / CI flag for ensuring all tests are run?
Posted by GitBox <gi...@apache.org>.
assignUser commented on code in PR #12940:
URL: https://github.com/apache/arrow/pull/12940#discussion_r855407167
##########
r/tests/testthat/helper-skip.R:
##########
@@ -21,7 +21,10 @@ build_features <- c(
uncompressed = TRUE
)
+force_tests <- identical(tolower(Sys.getenv("ARROW_R_FORCE_TESTS")), "true")
Review Comment:
I'll turn it into a function in case we move the helper stuff somewhere else at some point...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on a diff in pull request #12940: ARROW-15015: [R] Test / CI flag for ensuring all tests are run?
Posted by GitBox <gi...@apache.org>.
jonkeane commented on code in PR #12940:
URL: https://github.com/apache/arrow/pull/12940#discussion_r855398680
##########
r/tests/testthat/helper-skip.R:
##########
@@ -69,12 +76,14 @@ skip_on_linux_devel <- function() {
}
skip_if_r_version <- function(r_version) {
+ if (force_tests) return()
if (getRversion() <= r_version) {
skip(paste("R version:", getRversion()))
}
}
process_is_running <- function(x) {
+ if (force_tests) return(TRUE)
Review Comment:
A comment here about what is going on here would be nice, since it's slightly different from other places
##########
r/tests/testthat/helper-skip.R:
##########
@@ -21,7 +21,10 @@ build_features <- c(
uncompressed = TRUE
)
+force_tests <- identical(tolower(Sys.getenv("ARROW_R_FORCE_TESTS")), "true")
+
skip_if_not_available <- function(feature) {
+ if (force_tests) return()
Review Comment:
This is a minor style thing, but if there's an early return, we should make sure that goes on a separate line:
```suggestion
if (force_tests) {
return()
}
```
##########
r/tests/testthat/helper-skip.R:
##########
@@ -21,7 +21,10 @@ build_features <- c(
uncompressed = TRUE
)
+force_tests <- identical(tolower(Sys.getenv("ARROW_R_FORCE_TESTS")), "true")
Review Comment:
This is probably fine like it is, but if we wrapped this in a function, it would behave more like we expect — we wouldn't need to think about when this line is sourced
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] assignUser commented on pull request #12940: ARROW-15015: [R] Test / CI flag for ensuring all tests are run?
Posted by GitBox <gi...@apache.org>.
assignUser commented on PR #12940:
URL: https://github.com/apache/arrow/pull/12940#issuecomment-1106511616
It is merged an I rebaswd, I think the failures are due to the git cve or something? I'll check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane closed pull request #12940: ARROW-15015: [R] Test / CI flag for ensuring all tests are run?
Posted by GitBox <gi...@apache.org>.
jonkeane closed pull request #12940: ARROW-15015: [R] Test / CI flag for ensuring all tests are run?
URL: https://github.com/apache/arrow/pull/12940
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #12940: ARROW-15015: [R] Test / CI flag for ensuring all tests are run?
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #12940:
URL: https://github.com/apache/arrow/pull/12940#issuecomment-1110294320
Benchmark runs are scheduled for baseline = a16be6b7b6c8271202ff766b99c199b2e29bdfa8 and contender = e1e782a4542817e8a6139d6d5e022b56abdbc81d. e1e782a4542817e8a6139d6d5e022b56abdbc81d is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/80091439fa1e49769f50f0898250d13a...9af84758685c4d269ffd4cd0abea3fa9/)
[Failed] [test-mac-arm](https://conbench.ursa.dev/compare/runs/87b967bd821e45a1ba84127957feea6c...20b2367ab2454eeba717dff7af1b2ff5/)
[Failed :arrow_down:0.75% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/a01472b10d0a417384611e087cb84107...8b8a13ad985c4adfbf68518fb9fca07d/)
[Finished :arrow_down:0.13% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/6a0cafcf427d48e3b8ee3d9605e91d5b...a0be24cac1024327a4fa3340d17e9776/)
Buildkite builds:
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/595| `e1e782a4` ec2-t3-xlarge-us-east-2>
[Failed] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/583| `e1e782a4` test-mac-arm>
[Failed] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/582| `e1e782a4` ursa-i9-9960x>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/593| `e1e782a4` ursa-thinkcentre-m75q>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/594| `a16be6b7` ec2-t3-xlarge-us-east-2>
[Failed] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/582| `a16be6b7` test-mac-arm>
[Failed] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/581| `a16be6b7` ursa-i9-9960x>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/592| `a16be6b7` ursa-thinkcentre-m75q>
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org