You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by willb <gi...@git.apache.org> on 2014/07/07 23:17:27 UTC
[GitHub] spark pull request: Fix (some of the) warnings in the test suite
GitHub user willb opened a pull request:
https://github.com/apache/spark/pull/1323
Fix (some of the) warnings in the test suite
This PR fixes three classes of compiler and deprecation warnings in the test suite:
* `expectResult` is currently deprecated and has been replaced with `assertResult`;
* assertions of the form `should be ===` are deprecated in favor of the form `shouldEqual`; and
* `scala.language.postfixOps` was not in scope within the test classes in which postfix operations were actually used.
The fixes for these issues were almost entirely mechanical, but they eliminate many lines of warning output.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/willb/spark testCleanups
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/1323.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 #1323
----
commit b05304666eb61d7d1d1849772c27cf9711699dae
Author: William Benton <wi...@redhat.com>
Date: 2014-07-07T20:31:27Z
Replace expectResult with assertResult
Assertions.expectResult is deprecated. This commit replaces
expectResult invocations with the assertResult invocations.
commit 5377534fdfe741aab7d6d539573ab78636bfd554
Author: William Benton <wi...@redhat.com>
Date: 2014-07-07T20:37:53Z
Replaced deprecated 'be ===' assertions
"should be ===" assertions have been deprecated in favor of
"shouldEqual" assertions. This commit replaces the deprecated
form with the supported form.
commit a672a9b5d579c4946a20fe4c6f88c4582949ef3f
Author: William Benton <wi...@redhat.com>
Date: 2014-07-07T21:00:58Z
Ensure language.postfixOps is in scope where used
Previously, language.postfixOps was imported at toplevel, which meant
compiler warnings since it wasn't visible inside the classes that used
postfix operations. This commit moves the import to suppress these
warnings.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Fix postfixOps warnings in the test suite
Posted by willb <gi...@git.apache.org>.
Github user willb commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-55345856
Hi @jkbradley; thanks for taking a look. Here are the warnings as I see them when compiling tests on the immediate ancestor of my branch, which is 56e009d (I'm running on OS X 10.9 in this case):
```
[warn] /Users/willb/Documents/src/scala/alternate-spark/core/src/test/scala/org/apache/spark/ContextCleanerSuite.scala:38: postfix operator millis should be enabled
[warn] by making the implicit value scala.language.postfixOps visible.
[warn] This can be achieved by adding the import clause 'import scala.language.postfixOps'
[warn] or by setting the compiler option -language:postfixOps.
[warn] See the Scala docs for value scala.language.postfixOps for a discussion
[warn] why the feature should be explicitly enabled.
[warn] implicit val defaultTimeout = timeout(10000 millis)
[warn] ^
[warn] /Users/willb/Documents/src/scala/alternate-spark/core/src/test/scala/org/apache/spark/ContextCleanerSuite.scala:99: postfix operator millis should be enabled
[warn] by making the implicit value scala.language.postfixOps visible.
[warn] preGCTester.assertCleanup()(timeout(1000 millis))
[warn] ^
[warn] /Users/willb/Documents/src/scala/alternate-spark/core/src/test/scala/org/apache/spark/ContextCleanerSuite.scala:117: postfix operator millis should be enabled
[warn] by making the implicit value scala.language.postfixOps visible.
[warn] preGCTester.assertCleanup()(timeout(1000 millis))
[warn] ^
[warn] /Users/willb/Documents/src/scala/alternate-spark/core/src/test/scala/org/apache/spark/ContextCleanerSuite.scala:134: postfix operator millis should be enabled
[warn] by making the implicit value scala.language.postfixOps visible.
[warn] preGCTester.assertCleanup()(timeout(1000 millis))
[warn] ^
[warn] /Users/willb/Documents/src/scala/alternate-spark/core/src/test/scala/org/apache/spark/ContextCleanerSuite.scala:156: postfix operator millis should be enabled
[warn] by making the implicit value scala.language.postfixOps visible.
[warn] preGCTester.assertCleanup()(timeout(1000 millis))
[warn] ^
[warn] /Users/willb/Documents/src/scala/alternate-spark/core/src/test/scala/org/apache/spark/ContextCleanerSuite.scala:187: postfix operator millis should be enabled
[warn] by making the implicit value scala.language.postfixOps visible.
[warn] preGCTester.assertCleanup()(timeout(1000 millis))
[warn] ^
[warn] /Users/willb/Documents/src/scala/alternate-spark/core/src/test/scala/org/apache/spark/ContextCleanerSuite.scala:290: postfix operator millis should be enabled
[warn] by making the implicit value scala.language.postfixOps visible.
[warn] eventually(waitTimeout, interval(100 millis)) {
[warn] ^
[warn] /Users/willb/Documents/src/scala/alternate-spark/core/src/test/scala/org/apache/spark/rdd/AsyncRDDActionsSuite.scala:138: postfix operator seconds should be enabled
[warn] by making the implicit value scala.language.postfixOps visible.
[warn] failAfter(10 seconds) {
[warn] ^
[warn] /Users/willb/Documents/src/scala/alternate-spark/core/src/test/scala/org/apache/spark/rdd/AsyncRDDActionsSuite.scala:174: postfix operator seconds should be enabled
[warn] by making the implicit value scala.language.postfixOps visible.
[warn] failAfter(10 seconds) {
[warn] ^
[warn] /Users/willb/Documents/src/scala/alternate-spark/core/src/test/scala/org/apache/spark/ui/UISuite.scala:42: postfix operator seconds should be enabled
[warn] by making the implicit value scala.language.postfixOps visible.
[warn] eventually(timeout(10 seconds), interval(50 milliseconds)) {
[warn] ^
[warn] /Users/willb/Documents/src/scala/alternate-spark/core/src/test/scala/org/apache/spark/ui/UISuite.scala:42: postfix operator milliseconds should be enabled
[warn] by making the implicit value scala.language.postfixOps visible.
[warn] eventually(timeout(10 seconds), interval(50 milliseconds)) {
[warn] ^
[warn] /Users/willb/Documents/src/scala/alternate-spark/core/src/test/scala/org/apache/spark/ui/UISuite.scala:56: postfix operator seconds should be enabled
[warn] by making the implicit value scala.language.postfixOps visible.
[warn] eventually(timeout(10 seconds), interval(50 milliseconds)) {
[warn] ^
[warn] /Users/willb/Documents/src/scala/alternate-spark/core/src/test/scala/org/apache/spark/ui/UISuite.scala:56: postfix operator milliseconds should be enabled
[warn] by making the implicit value scala.language.postfixOps visible.
[warn] eventually(timeout(10 seconds), interval(50 milliseconds)) {
[warn] ^
[warn] /Users/willb/Documents/src/scala/alternate-spark/core/src/test/scala/org/apache/spark/ui/UISuite.scala:75: postfix operator seconds should be enabled
[warn] by making the implicit value scala.language.postfixOps visible.
[warn] eventually(timeout(10 seconds), interval(50 milliseconds)) {
[warn] ^
[warn] /Users/willb/Documents/src/scala/alternate-spark/core/src/test/scala/org/apache/spark/ui/UISuite.scala:75: postfix operator milliseconds should be enabled
[warn] by making the implicit value scala.language.postfixOps visible.
[warn] eventually(timeout(10 seconds), interval(50 milliseconds)) {
[warn] ^
[warn] /Users/willb/Documents/src/scala/alternate-spark/core/src/test/scala/org/apache/spark/ui/UISuite.scala:89: postfix operator seconds should be enabled
[warn] by making the implicit value scala.language.postfixOps visible.
[warn] eventually(timeout(10 seconds), interval(50 milliseconds)) {
[warn] ^
[warn] /Users/willb/Documents/src/scala/alternate-spark/core/src/test/scala/org/apache/spark/ui/UISuite.scala:89: postfix operator milliseconds should be enabled
[warn] by making the implicit value scala.language.postfixOps visible.
[warn] eventually(timeout(10 seconds), interval(50 milliseconds)) {
```
Moving the import to within the test class (as in my patch) eliminated the warnings.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: Fix (some of the) warnings in the test suite
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-48242783
Merged build triggered.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Fix (some of the) warnings in the test suite
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-48326263
Merged build triggered.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Fix (some of the) warnings in the test suite
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-48326270
Merged build started.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Fix postfixOps warnings in the test suite
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-55352231
I see, thanks for your patch anyway!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: Fix postfixOps warnings in the test suite
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-51076281
QA results for PR 1323:<br>- This patch PASSES unit tests.<br>- This patch merges cleanly<br>- This patch adds no public classes<br><br>For more information see test ouptut:<br>https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17854/consoleFull
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: Fix postfixOps warnings in the test suite
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-51068459
QA tests have started for PR 1323. This patch merges cleanly. <br>View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17854/consoleFull
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: Fix (some of the) warnings in the test suite
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-48253089
Merged build finished. All automated tests passed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Fix (some of the) warnings in the test suite
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-48242653
This covers a lot of the same ground as https://github.com/apache/spark/pull/1153 . It would be great to get all of this in to stop the warnings.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Fix (some of the) warnings in the test suite
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-48242803
Merged build started.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Fix postfixOps warnings in the test suite
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-48329997
Merged build finished. All automated tests passed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Fix (some of the) warnings in the test suite
Posted by willb <gi...@git.apache.org>.
Github user willb commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-48326403
@rxin Done; I also updated the comment to reflect the narrower focus after eliminating overlap with #1153. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Fix (some of the) warnings in the test suite
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-48253092
All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16385/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Fix postfixOps warnings in the test suite
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-48329998
All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16407/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Fix (some of the) warnings in the test suite
Posted by willb <gi...@git.apache.org>.
Github user willb commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-48243538
@srowen sorry, I hadn't noticed the other PR! I think the `postfixOps` change in mine is disjoint, though.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Fix postfixOps warnings in the test suite
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-55347991
Hi @wllib can you comment on #1330? What is the difference between that patch and this, and which is the better approach?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: Fix postfixOps warnings in the test suite
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-53340472
@willb @witgo What is the difference between your patches?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: Fix postfixOps warnings in the test suite
Posted by willb <gi...@git.apache.org>.
Github user willb commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-55351655
Hey @andrewor14, thanks for the reply. First off, I absolutely agree with @srowen's comment on #1330 that `import`s (not compiler flags) are the right way to handle enabling these language features. It looks to me like `SpanSugar` pulls in `postfixOps` -- and that it's the only thing in those files that uses `postfixOps`. (I guess having both `SpanSugar` and `postfixOps` imported at toplevel was causing some implicit resolution confusion?)
In any case, the approach in #1330 is probably the way to go since explicitly importing `postfixOps` seems unnecessary and removing the compiler flag is a good idea. Thanks again for taking a look!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: Fix (some of the) warnings in the test suite
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-48280759
I just merged #1153 and unfortunately this one is no longer mergeable. Will - mind updating it?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Fix postfixOps warnings in the test suite
Posted by witgo <gi...@git.apache.org>.
Github user witgo commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-51069490
Related work #1330
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: Fix postfixOps warnings in the test suite
Posted by willb <gi...@git.apache.org>.
Github user willb closed the pull request at:
https://github.com/apache/spark/pull/1323
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: Fix postfixOps warnings in the test suite
Posted by willb <gi...@git.apache.org>.
Github user willb commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-51378794
Can someone take a look at this again?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: Fix postfixOps warnings in the test suite
Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the pull request:
https://github.com/apache/spark/pull/1323#issuecomment-55194405
@willb Hi, I was asked to take a look at this. I tried the current master + this PR merged with master, and I did not see much difference in warnings (or any warnings about postfixOps). Could you please show the warnings, and info which might indicate the system/setup which produces the warnings? Thank you!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org