You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@kudu.apache.org by "Attila Bukor (Code Review)" <ge...@cloudera.org> on 2020/04/07 14:33:40 UTC

[kudu-CR] KUDU-3078 Refactor Sentry integration tests

Hello Andrew Wong, Adar Dembo, Grant Henke, Hao Hao,

I'd like you to do a code review. Please visit

    http://gerrit.cloudera.org:8080/15671

to review the following change.


Change subject: KUDU-3078 Refactor Sentry integration tests
......................................................................

KUDU-3078 Refactor Sentry integration tests

This commit refactors the existing Sentry integration tests to prepare
for Ranger integration tests.

It changes HMS and Sentry integration test base classes to test
harnesses that don't inherit from Gtest to simplify inheritance when
typed tests.

Change-Id: Ieff25eaf7a41cf7d8fe965738907e67bbd9b9051
---
M src/kudu/integration-tests/external_mini_cluster-itest-base.h
M src/kudu/integration-tests/hms_itest-base.cc
M src/kudu/integration-tests/hms_itest-base.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/integration-tests/master_sentry-itest.cc
M src/kudu/integration-tests/ts_sentry-itest.cc
6 files changed, 456 insertions(+), 190 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/71/15671/1
-- 
To view, visit http://gerrit.cloudera.org:8080/15671
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ieff25eaf7a41cf7d8fe965738907e67bbd9b9051
Gerrit-Change-Number: 15671
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>

[kudu-CR] KUDU-3078 Refactor Sentry integration tests

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.

Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15671 )

Change subject: KUDU-3078 Refactor Sentry integration tests
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15671/2/src/kudu/integration-tests/master_hms-itest.cc
File src/kudu/integration-tests/master_hms-itest.cc:

http://gerrit.cloudera.org:8080/#/c/15671/2/src/kudu/integration-tests/master_hms-itest.cc@80
PS2, Line 80:     hms_opts.enable_kerberos = EnableKerberos();
            :     hms_opts.service_principal = "hive";
            :     harness_.hms_client_.reset(new HmsC
> hm maybe I'm missing something, but how would I reset hms_client_ if I acce
It probably wouldn't hurt to move this into the harness itself with some HmsITestHarness::ResetHmsClient(address, opts) or somesuch.


http://gerrit.cloudera.org:8080/#/c/15671/2/src/kudu/integration-tests/master_sentry-itest.cc
File src/kudu/integration-tests/master_sentry-itest.cc:

http://gerrit.cloudera.org:8080/#/c/15671/2/src/kudu/integration-tests/master_sentry-itest.cc@414
PS2, Line 414:                             
> maybe I misunderstood what you're suggesting, but wouldn't that be duplicat
Would something like this work?

SentryITestHarness {
  /* looks like it does now for the most part */
  virtual ExternalMiniClusterOptions GetClusterOpts() {
    /* opts we have in SetUpCluster */
  }
  void SetUpCluster() {
    ...
    SetUpClusterWithOpts(GetClusterOpts());
    ...
  }
};

SentryITestHarnessWithCache : public SentryITestHarness {
  ExternalMiniClusterOptions GetClusterOpts() override {
    ExternalMiniClusterOptions opts = SentryITestHarness::GetClusterOpts();
    // add cache gflag
    return opts;
  }
};

It's a little more legwork, but it helps decouple the harness and test a bit, which I find appealing.



-- 
To view, visit http://gerrit.cloudera.org:8080/15671
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieff25eaf7a41cf7d8fe965738907e67bbd9b9051
Gerrit-Change-Number: 15671
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 07 Apr 2020 21:08:58 +0000
Gerrit-HasComments: Yes