You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org> on 2017/02/13 19:24:32 UTC
[kudu-CR] Add a helper method to determine whether the OS is macOS to KuduTest
David Ribeiro Alves has uploaded a new change for review.
http://gerrit.cloudera.org:8080/5981
Change subject: Add a helper method to determine whether the OS is macOS to KuduTest
......................................................................
Add a helper method to determine whether the OS is macOS to KuduTest
Some tests simply can't be run on macOs. A specific case are the tests
that depend on obtaining the clock error. We were previously making
the clock "lie" and return an error of 0. This might lead to incorrect
results. This patch adds a helper that we can use to simply skip
the tests on macOS without having to add ifdefs.
Change-Id: I9e60ec4ddaf5aa21d1d8f6e848c211f75ded6ba0
---
M src/kudu/util/test_util.cc
M src/kudu/util/test_util.h
2 files changed, 10 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/5981/1
--
To view, visit http://gerrit.cloudera.org:8080/5981
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9e60ec4ddaf5aa21d1d8f6e848c211f75ded6ba0
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
[kudu-CR] Add a helper method to determine whether the OS is macOS to KuduTest
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: Add a helper method to determine whether the OS is macOS to KuduTest
......................................................................
Patch Set 3:
Is this still needed? Just wanted to check in on status before it gets really stale
--
To view, visit http://gerrit.cloudera.org:8080/5981
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9e60ec4ddaf5aa21d1d8f6e848c211f75ded6ba0
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] Add a helper method to determine whether the OS is macOS to KuduTest
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: Add a helper method to determine whether the OS is macOS to KuduTest
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/5981
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9e60ec4ddaf5aa21d1d8f6e848c211f75ded6ba0
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] Add a helper method to determine whether the OS is macOS to KuduTest
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: Add a helper method to determine whether the OS is macOS to KuduTest
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5981/1/src/kudu/util/test_util.h
File src/kudu/util/test_util.h:
Line 63: // Returns whether the platform is macOS/mac OSX.
> Nit: if you want to be even more precise, it's "macOS" or "Mac OS X".
Done
--
To view, visit http://gerrit.cloudera.org:8080/5981
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9e60ec4ddaf5aa21d1d8f6e848c211f75ded6ba0
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Add a helper method to determine whether the OS is macOS to KuduTest
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Adar Dembo, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5981
to look at the new patch set (#3).
Change subject: Add a helper method to determine whether the OS is macOS to KuduTest
......................................................................
Add a helper method to determine whether the OS is macOS to KuduTest
Some tests simply can't be run on macOS. A specific case are the tests
that depend on obtaining the clock error. We were previously making
the clock "lie" and return an error of 0. This might lead to incorrect
results. This patch adds a helper that we can use to simply skip
the tests on macOS without having to add ifdefs.
Change-Id: I9e60ec4ddaf5aa21d1d8f6e848c211f75ded6ba0
---
M src/kudu/util/test_util.cc
M src/kudu/util/test_util.h
2 files changed, 10 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/5981/3
--
To view, visit http://gerrit.cloudera.org:8080/5981
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9e60ec4ddaf5aa21d1d8f6e848c211f75ded6ba0
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Add a helper method to determine whether the OS is macOS to KuduTest
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Adar Dembo, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5981
to look at the new patch set (#2).
Change subject: Add a helper method to determine whether the OS is macOS to KuduTest
......................................................................
Add a helper method to determine whether the OS is macOS to KuduTest
Some tests simply can't be run on macOs. A specific case are the tests
that depend on obtaining the clock error. We were previously making
the clock "lie" and return an error of 0. This might lead to incorrect
results. This patch adds a helper that we can use to simply skip
the tests on macOS without having to add ifdefs.
Change-Id: I9e60ec4ddaf5aa21d1d8f6e848c211f75ded6ba0
---
M src/kudu/util/test_util.cc
M src/kudu/util/test_util.h
2 files changed, 10 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/81/5981/2
--
To view, visit http://gerrit.cloudera.org:8080/5981
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9e60ec4ddaf5aa21d1d8f6e848c211f75ded6ba0
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Add a helper method to determine whether the OS is macOS to KuduTest
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: Add a helper method to determine whether the OS is macOS to KuduTest
......................................................................
Patch Set 3: Verified+1
unrelated flake
--
To view, visit http://gerrit.cloudera.org:8080/5981
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9e60ec4ddaf5aa21d1d8f6e848c211f75ded6ba0
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] Add a helper method to determine whether the OS is macOS to KuduTest
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: Add a helper method to determine whether the OS is macOS to KuduTest
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
I don't know. I still prefer disabling functionality (even in tests) based on capability rather than outright version checks. But, I don't have a strong opinion.
http://gerrit.cloudera.org:8080/#/c/5981/1/src/kudu/util/test_util.h
File src/kudu/util/test_util.h:
Line 63: // Returns whether the platform is macOS/mac OSX.
Nit: if you want to be even more precise, it's "macOS" or "Mac OS X".
--
To view, visit http://gerrit.cloudera.org:8080/5981
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9e60ec4ddaf5aa21d1d8f6e848c211f75ded6ba0
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes