You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by cloud-fan <gi...@git.apache.org> on 2018/01/18 06:51:30 UTC
[GitHub] spark pull request #20310: revert [SPARK-10030] Use tags to control which te...
GitHub user cloud-fan opened a pull request:
https://github.com/apache/spark/pull/20310
revert [SPARK-10030] Use tags to control which tests to run depending on changes being tested
## What changes were proposed in this pull request?
This PR reverts https://github.com/apache/spark/pull/8775
The problem is that, when we change the code in Spark core, we may break yarn test, so we should run yarn test even we didn't change code in the yarn module. We broke the build because of this issue in https://github.com/apache/spark/pull/20223
## How was this patch tested?
N/A
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cloud-fan/spark test
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20310.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #20310
----
commit 47687bb37b9aaeac7e0305c0eaa7f419255e1a45
Author: Wenchen Fan <we...@...>
Date: 2018-01-18T06:42:57Z
rever [SPARK-10030] Use tags to control which tests to run depending on changes being tested
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20310: revert [SPARK-10030] Use tags to control which tests to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20310
**[Test build #86325 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86325/testReport)** for PR 20310 at commit [`ac77bb4`](https://github.com/apache/spark/commit/ac77bb4dee17aec2ed7afd2138115947a8eef923).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20310: revert [SPARK-10030] Use tags to control which tests to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20310
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86321/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20310: revert [SPARK-10030] Use tags to control which tests to ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/20310
To answer one of Josh's question, from the link above, it seems the YARN integration tests currently add ~5m to the PRB time. That doesn't sound too horrible, but would potentially make current timeout issues worse since we seem to be brushing against the existing limit already.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20310: revert [SPARK-10030] Use tags to control which te...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20310#discussion_r162265397
--- Diff: common/tags/src/test/java/org/apache/spark/tags/DockerTest.java ---
@@ -1,26 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.spark.tags;
-
-import java.lang.annotation.*;
-import org.scalatest.TagAnnotation;
-
-@TagAnnotation
-@Retention(RetentionPolicy.RUNTIME)
-@Target({ElementType.METHOD, ElementType.TYPE})
-public @interface DockerTest { }
--- End diff --
We don't use this docker tag in `modules.py`, does it mean we never run docker tests in PR builder?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20310: revert [SPARK-10030] Use tags to control which tests to ...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the issue:
https://github.com/apache/spark/pull/20310
Are you sure that we want to blanket revert this entire patch? Is there a more surgical short-term fix we can make in `dev/sparktestsupport/modules.py` to just always unconditionally enable the tag for now?
Also, is this the first time recently that we've failed the YARN integration tests? How much time do they add?
The trade off here seems to be between slightly slower after-the-fact detection of a test failure / build break due to YARN vs. faster tests for the majority of PRs that don't touch YARN code. I think we've had one or two such breaks in the 2+ years that we've been using these test tags, so I'd also be fine with postponing this change if you agree that it's unlikely that we're going to have many such failures here.
If the motivation is that it's hard to test the fix for such build breaks (because the failing test wouldn't be exercised in the PR builder) then I think we might already have a solution via special tags placed into the PR title (I think `test-yarn` or something similar; see `run-tests-jenkins.py`).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20310: revert [SPARK-10030] Use tags to control which tests to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20310
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20310: revert [SPARK-10030] Use tags to control which tests to ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20310
We want a way to test yarn for the new fix: https://github.com/apache/spark/pull/20297
I checked `run-tests-jenkins.py` and seems we only support `test-maven`, `test-hadoop2.6` and `test-hadoop2.6`. It will be good if we have a `test-yarn` and then we don't need this patch.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20310: revert [SPARK-10030] Use tags to control which te...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/20310#discussion_r162268244
--- Diff: common/tags/src/test/java/org/apache/spark/tags/DockerTest.java ---
@@ -1,26 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.spark.tags;
-
-import java.lang.annotation.*;
-import org.scalatest.TagAnnotation;
-
-@TagAnnotation
-@Retention(RetentionPolicy.RUNTIME)
-@Target({ElementType.METHOD, ElementType.TYPE})
-public @interface DockerTest { }
--- End diff --
If you search through the commit history, I'm pretty sure that we originally tried running those DockerTests on RISELAB Jenkins but ran into problems with the Docker Daemon becoming unstable under heavy build load. This should be fixed in the newer-generation Ubuntu build workers, but we haven't quite finished migrating the PRBs onto those.
Given this, my hunch is that those tests aren't running anywhere right now, which isn't great. I think they're primarily used for testing JDBC data source SQL dialect mappings. It's been a year or more since I last looked into this, though, so I might be misremembering.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20310: revert [SPARK-10030] Use tags to control which tests to ...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20310
cc @vanzin @sameeragarwal
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20310: revert [SPARK-10030] Use tags to control which tests to ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/20310
@cloud-fan
> We want a way to test yarn for the new fix: 20297
In that case we're ok because that fix touches YARN code, so YARN tests are running.
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86289/testReport/org.apache.spark.deploy.yarn/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20310: revert [SPARK-10030] Use tags to control which tests to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20310
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20310: revert [SPARK-10030] Use tags to control which tests to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20310
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86325/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20310: revert [SPARK-10030] Use tags to control which te...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/20310#discussion_r162266565
--- Diff: common/tags/src/test/java/org/apache/spark/tags/DockerTest.java ---
@@ -1,26 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.spark.tags;
-
-import java.lang.annotation.*;
-import org.scalatest.TagAnnotation;
-
-@TagAnnotation
-@Retention(RetentionPolicy.RUNTIME)
-@Target({ElementType.METHOD, ElementType.TYPE})
-public @interface DockerTest { }
--- End diff --
if it is, we should preserve this behavior.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20310: revert [SPARK-10030] Use tags to control which te...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan closed the pull request at:
https://github.com/apache/spark/pull/20310
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20310: revert [SPARK-10030] Use tags to control which tests to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20310
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20310: revert [SPARK-10030] Use tags to control which tests to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20310
**[Test build #86321 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86321/testReport)** for PR 20310 at commit [`47687bb`](https://github.com/apache/spark/commit/47687bb37b9aaeac7e0305c0eaa7f419255e1a45).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20310: revert [SPARK-10030] Use tags to control which tests to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20310
**[Test build #86321 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86321/testReport)** for PR 20310 at commit [`47687bb`](https://github.com/apache/spark/commit/47687bb37b9aaeac7e0305c0eaa7f419255e1a45).
* This patch **fails to build**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20310: revert [SPARK-10030] Use tags to control which tests to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20310
**[Test build #86333 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86333/testReport)** for PR 20310 at commit [`b6c46b5`](https://github.com/apache/spark/commit/b6c46b5900bf1109f836674b8ba5ee3cb4712771).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20310: revert [SPARK-10030] Use tags to control which tests to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20310
**[Test build #86333 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86333/testReport)** for PR 20310 at commit [`b6c46b5`](https://github.com/apache/spark/commit/b6c46b5900bf1109f836674b8ba5ee3cb4712771).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20310: revert [SPARK-10030] Use tags to control which tests to ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20310
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86333/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20310: revert [SPARK-10030] Use tags to control which tests to ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20310
**[Test build #86325 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86325/testReport)** for PR 20310 at commit [`ac77bb4`](https://github.com/apache/spark/commit/ac77bb4dee17aec2ed7afd2138115947a8eef923).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org