You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "David Knupp (Code Review)" <ge...@cloudera.org> on 2016/12/09 23:02:25 UTC

[Impala-ASF-CR] Add pytest option and skip markers for tests that only run locally.

David Knupp has uploaded a new change for review.

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

Change subject: Add pytest option and skip markers for tests that only run locally.
......................................................................

Add pytest option and skip markers for tests that only run locally.

Add new skip marker to test_mt_dop.TestMtDopParquet.test_parquet_nested

Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
---
M tests/common/skip.py
M tests/conftest.py
M tests/query_test/test_mt_dop.py
3 files changed, 11 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/5446/1
-- 
To view, visit http://gerrit.cloudera.org:8080/5446
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>

[Impala-ASF-CR] Add pytest option and skip markers for tests that only run locally.

Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change.

Change subject: Add pytest option and skip markers for tests that only run locally.
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5446/1//COMMIT_MSG
Commit Message:

PS1, Line 7: Add pytest option and skip markers for tests that only run locally.
Please add Jira ID.


http://gerrit.cloudera.org:8080/#/c/5446/1/tests/common/skip.py
File tests/common/skip.py:

PS1, Line 120: class SkipIfRemoteTesting:
             :   """Skip marker for those tests that may not work on remote clusters."""
             :   local_cluster_only = pytest.mark.skipif(pytest.config.option.skip_local_only_tests,
             :       reason="--skip_local_only_tests specified")
I'm fine with the class name. I'm less enthused on the command line name. We already have one notion of "local", which is the local filesystem as opposed to HDFS et al. Now we are introducing another notion of "local". Is there a philosophical argument I'm missing against something like "--remote-cluster-tests" ? That is more precise and clear as to the nature of the testing.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
Hello Michael Brown, Lars Volker,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#4).

Change subject: IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.
......................................................................

IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.

As we're beginning to run Impala end-to-end tests on remote clusters, we're
finding some tests that do not pass for infrastructure-related reasons (as
opposed to product issues.) It would be useful to be able to xfail any tests
that we know to be problematic within a given module, yet still run the
others. This way, we can get passing test runs as we're ironing out those
infrastructure issues.

Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
---
M tests/conftest.py
M tests/metadata/test_compute_stats.py
M tests/query_test/test_mt_dop.py
3 files changed, 24 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/5446/4
-- 
To view, visit http://gerrit.cloudera.org:8080/5446
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.

Change subject: IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/5446/3/tests/conftest.py
File tests/conftest.py:

Line 480:   Validate that pytest command line options makes sense.
> typo: make sense
Done


Line 483:     if "localhost" in pytest.config.option.impalad:
> Any address starting with 127.x.x.x is the local machine I think.
Done


Line 483:     if "localhost" in pytest.config.option.impalad:
> or "127.0.0.1" ?
Done


http://gerrit.cloudera.org:8080/#/c/5446/3/tests/metadata/test_compute_stats.py
File tests/metadata/test_compute_stats.py:

Line 135:                      reason=("Because of the way we pre-split the data, setting up HBase "
> add JIRA to the reason, so we can easily fix all the places later if needed
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.

Change subject: IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/5446/3/tests/conftest.py
File tests/conftest.py:

Line 480:   Validate that pytest command line options makes sense.
typo: make sense


Line 483:     if "localhost" in pytest.config.option.impalad:
or "127.0.0.1" ?


http://gerrit.cloudera.org:8080/#/c/5446/3/tests/metadata/test_compute_stats.py
File tests/metadata/test_compute_stats.py:

Line 135:                      reason=("Because of the way we pre-split the data, setting up HBase "
add JIRA to the reason, so we can easily fix all the places later if needed


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change.

Change subject: IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5446/3/tests/conftest.py
File tests/conftest.py:

Line 483:     if "localhost" in pytest.config.option.impalad:
> or "127.0.0.1" ?
Any address starting with 127.x.x.x is the local machine I think.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Add pytest option and skip markers for tests that only run locally.

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: Add pytest option and skip markers for tests that only run locally.
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5446/1/tests/query_test/test_mt_dop.py
File tests/query_test/test_mt_dop.py:

PS1, Line 95: @SkipIfRemoteTesting.local_cluster_only
> From just looking at this it is not clear to me whether this is skipped or 
I think also it's confusing when we have the distinction between a local HDFS cluster and local filesystem.  Maybe this could be minicluster_only?


Line 99:     self.run_test_case('QueryTest/mt-dop-parquet-nested', vector)
Why isn't this test expected to succeed on a cluster? It seems like it should work unless there's a product bug.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4639: Add pytest option and skip markers for tests that only run locally.

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.

Change subject: IMPALA-4639: Add pytest option and skip markers for tests that only run locally.
......................................................................


Patch Set 2:

(4 comments)

Is the motivation of this change to get Jenkins jobs run against a remote cluster green? Please briefly summarize the motivation of this change.

http://gerrit.cloudera.org:8080/#/c/5446/2/tests/common/skip.py
File tests/common/skip.py:

Line 122:   mini_cluster_only = pytest.mark.skipif(pytest.config.option.testing_remote_cluster,
Why skip and not xfail? With xfail we have the option of still running the tests, e.g., to detect crashes.


http://gerrit.cloudera.org:8080/#/c/5446/2/tests/conftest.py
File tests/conftest.py:

Line 108:   parser.addoption("--testing_remote_cluster", action="store_true", default=False,
I'm ok with adding this flag, but I think we should enforce its semantics. Currently, we can set this flag regardless of whether we are testing on a remote cluster or not which seems potentially error-prone.


Line 109:                    help="Indicates that tests are being run against a remote cluster.")
Mention what effect this flag has. It will cause tests to be skipped.


http://gerrit.cloudera.org:8080/#/c/5446/2/tests/query_test/test_mt_dop.py
File tests/query_test/test_mt_dop.py:

Line 95:   @SkipIfRemoteTesting.mini_cluster_only
Does test_nested_types.py or test_tpch_nested_queries.py succeed on a remote cluster? I'm surprised that only this test would be skipped of loading tpch_nested is not working.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4639: Add pytest option and skip markers for tests that only run locally.

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change.

Change subject: IMPALA-4639: Add pytest option and skip markers for tests that only run locally.
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5446/2/tests/common/skip.py
File tests/common/skip.py:

PS2, Line 123: Setup for this t
Maybe better "This test is currently..."?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.

Change subject: IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.
......................................................................


Patch Set 5: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.

Change subject: IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.
......................................................................


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.
......................................................................


IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.

As we're beginning to run Impala end-to-end tests on remote clusters, we're
finding some tests that do not pass for infrastructure-related reasons (as
opposed to product issues.) It would be useful to be able to xfail any tests
that we know to be problematic within a given module, yet still run the
others. This way, we can get passing test runs as we're ironing out those
infrastructure issues.

Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Reviewed-on: http://gerrit.cloudera.org:8080/5446
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Internal Jenkins
---
M tests/conftest.py
M tests/metadata/test_compute_stats.py
M tests/query_test/test_mt_dop.py
3 files changed, 24 insertions(+), 0 deletions(-)

Approvals:
  Internal Jenkins: Verified
  Alex Behm: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-4639: Add pytest option and skip markers for tests that only run locally.

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has uploaded a new patch set (#2).

Change subject: IMPALA-4639: Add pytest option and skip markers for tests that only run locally.
......................................................................

IMPALA-4639: Add pytest option and skip markers for tests that only run locally.

Add new skip marker to test_mt_dop.TestMtDopParquet.test_parquet_nested

Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
---
M tests/common/skip.py
M tests/conftest.py
M tests/metadata/test_compute_stats.py
M tests/query_test/test_mt_dop.py
4 files changed, 13 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/5446/2
-- 
To view, visit http://gerrit.cloudera.org:8080/5446
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.

Change subject: IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.
......................................................................


Patch Set 5: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.

Change subject: IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/5446/2/tests/common/skip.py
File tests/common/skip.py:

Line 122
> Why skip and not xfail? With xfail we have the option of still running the 
I think switching to xfail is a great suggestion. As such, I'll remove the SkipIf class for now.


http://gerrit.cloudera.org:8080/#/c/5446/2/tests/conftest.py
File tests/conftest.py:

Line 108:   parser.addoption("--testing_remote_cluster", action="store_true", default=False,
> I'm ok with adding this flag, but I think we should enforce its semantics. 
I'm adding an autouse fixture to conftest.py to make sure that we don't allow --testing_remote_cluster when impalad ~ localhost. Let me know if you had something more sophisticated in mind.


Line 109:                    help=("Indicates that tests are being run against a remote cluster. "
> Mention what effect this flag has. It will cause tests to be skipped.
Done


http://gerrit.cloudera.org:8080/#/c/5446/2/tests/query_test/test_mt_dop.py
File tests/query_test/test_mt_dop.py:

Line 95:   @pytest.mark.xfail(pytest.config.option.testing_remote_cluster,
> Does test_nested_types.py or test_tpch_nested_queries.py succeed on a remot
Those tests don't pass yet either. We actually ruled those module out entirely until we can get to the root of the problem. However, there were only a couple of failing tests in this module, and it seemed like a good reason to introduce a temporary workaround so that we can still run the others.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4639: Add pytest option and skip markers for tests that only run locally.

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.

Change subject: IMPALA-4639: Add pytest option and skip markers for tests that only run locally.
......................................................................


Patch Set 1:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/5446/1//COMMIT_MSG
Commit Message:

PS1, Line 7: Add pytest option and skip markers for tests that only run locally.
> Please add Jira ID.
Done


http://gerrit.cloudera.org:8080/#/c/5446/1/tests/common/skip.py
File tests/common/skip.py:

PS1, Line 120: class SkipIfRemoteTesting:
             :   """Skip marker for those tests that may not work on remote clusters."""
             :   local_cluster_only = pytest.mark.skipif(pytest.config.option.skip_local_only_tests,
             :       reason="--skip_local_only_tests specified")
> I'm fine with the class name. I'm less enthused on the command line name. W
Done


PS1, Line 123: --skip_local_only_tests
> Could this ever be re-used by some other dev without that command line opti
No, if you don't specify the command line, then this won't have any effect.


PS1, Line 124: hbase_split
> Is this used anywhere?
Thanks, forgot to add that file where it gets used.


PS1, Line 125: HBase is not split on remote cluster the same as on the local cluster.
> This doesn't tell me much, probably because I don't know how splitting work
Hbase splitting is usually non-deterministic, but we enforce a particular split on our local tests via a setup shell script, and then conform our tests to expect that split. We don't have a way of doing this on the remote cluster, and it would require a big overhaul to our existing tests to work without it.


http://gerrit.cloudera.org:8080/#/c/5446/1/tests/query_test/test_mt_dop.py
File tests/query_test/test_mt_dop.py:

PS1, Line 95: @SkipIfRemoteTesting.local_cluster_only
> From just looking at this it is not clear to me whether this is skipped or 
This will only be skipped if --testing_remote_cluster is supplied on the command line.


PS1, Line 95: @SkipIfRemoteTesting.local_cluster_only
> I think also it's confusing when we have the distinction between a local HD
Done


Line 99:     self.run_test_case('QueryTest/mt-dop-parquet-nested', vector)
> Why isn't this test expected to succeed on a cluster? It seems like it shou
It's a setup problem. We're still working though kinks on getting our entire data load working across the board on remote clusters. Even though the process completes without any errors, here are as yet silent failures. I can add a JIRA and reference it here.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
Hello Michael Brown, Lars Volker,

I'd like you to reexamine a change.  Please visit

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

to look at the new patch set (#3).

Change subject: IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.
......................................................................

IMPALA-4639: Add pytest option and xfail markers for tests that only run locally.

As we're beginning to run Impala end-to-end tests on remote clusters, we're
finding some tests that do not pass for infrastructure-related reasons (as
opposed to product issues.) It would be useful to be able to xfail any tests
that we know to be problematic within a given module, yet still run the
others. This way, we can get passing test runs as we're ironing out those
infrastructure issues.

Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
---
M tests/conftest.py
M tests/metadata/test_compute_stats.py
M tests/query_test/test_mt_dop.py
3 files changed, 23 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/5446/3
-- 
To view, visit http://gerrit.cloudera.org:8080/5446
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] Add pytest option and skip markers for tests that only run locally.

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change.

Change subject: Add pytest option and skip markers for tests that only run locally.
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/5446/1/tests/common/skip.py
File tests/common/skip.py:

PS1, Line 123: --skip_local_only_tests
Could this ever be re-used by some other dev without that command line option being present? Then the error might be misleading.


PS1, Line 124: hbase_split
Is this used anywhere?


PS1, Line 125: HBase is not split on remote cluster the same as on the local cluster.
This doesn't tell me much, probably because I don't know how splitting works local and/or remote. Maybe you can rephrase to explain what exactly is different.


http://gerrit.cloudera.org:8080/#/c/5446/1/tests/query_test/test_mt_dop.py
File tests/query_test/test_mt_dop.py:

PS1, Line 95: @SkipIfRemoteTesting.local_cluster_only
From just looking at this it is not clear to me whether this is skipped or only runs locally / remotely.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4639: Add pytest option and skip markers for tests that only run locally.

Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change.

Change subject: IMPALA-4639: Add pytest option and skip markers for tests that only run locally.
......................................................................


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4d6e46dc1e64ad20c727ccb19af7a9f3daf917f
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No