Fix postfixOps warnings in the test suite by willb · Pull Request #1323 · apache/spark

willb · 2014-07-07T21:17:23Z

This PR fixes compiler warnings in the test suite related to scala.language.postfixOps not being in scope within the test classes in which postfix operations were actually used.

srowen · 2014-07-07T21:19:56Z

This covers a lot of the same ground as #1153 . It would be great to get all of this in to stop the warnings.

AmplabJenkins · 2014-07-07T21:21:04Z

Merged build triggered.

AmplabJenkins · 2014-07-07T21:21:12Z

Merged build started.

willb · 2014-07-07T21:27:48Z

@srowen sorry, I hadn't noticed the other PR! I think the postfixOps change in mine is disjoint, though.

AmplabJenkins · 2014-07-07T23:09:05Z

Merged build finished. All automated tests passed.

AmplabJenkins · 2014-07-07T23:09:06Z

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16385/

rxin · 2014-07-08T07:33:39Z

I just merged #1153 and unfortunately this one is no longer mergeable. Will - mind updating it?

AmplabJenkins · 2014-07-08T11:51:06Z

Merged build triggered.

AmplabJenkins · 2014-07-08T11:51:12Z

Merged build started.

willb · 2014-07-08T11:52:53Z

@rxin Done; I also updated the comment to reflect the narrower focus after eliminating overlap with #1153. Thanks!

AmplabJenkins · 2014-07-08T12:35:53Z

Merged build finished. All automated tests passed.

AmplabJenkins · 2014-07-08T12:35:54Z

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16407/

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.

SparkQA · 2014-08-04T14:39:18Z

QA tests have started for PR 1323. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17854/consoleFull

witgo · 2014-08-04T14:46:41Z

Related work #1330

SparkQA · 2014-08-04T15:35:57Z

QA results for PR 1323:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17854/consoleFull

willb · 2014-08-06T18:45:59Z

Can someone take a look at this again?

andrewor14 · 2014-08-25T21:50:38Z

@willb @witgo What is the difference between your patches? Are they the same?

jkbradley · 2014-09-10T22:42:34Z

@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!

willb · 2014-09-12T00:03:37Z

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.

andrewor14 · 2014-09-12T00:34:29Z

Hi @wllib can you comment on #1330? What is the difference between that patch and this, and which is the better approach? It seems to me that simply removing the postFixOps import is sufficient in eliminating the warnings.

willb · 2014-09-12T01:32:14Z

Hey @andrewor14, thanks for the reply. First off, I absolutely agree with @srowen's comment on #1330 that imports (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!

andrewor14 · 2014-09-12T01:42:38Z

I see, thanks for your patch anyway!

…ault (apache#1323) ### What changes were proposed in this pull request? This PR aims to add a new executor roll policy, `OUTLIER`, which aims to detect various outliers first, and use it by default. If there is no outlier, it will work like `TOTAL_DURATION` policy. ### Why are the changes needed? The users can use `OUTLIER` policy to consider the outliers in terms of multiple dimensions. In addition, this will be a better default policy. ### Does this PR introduce _any_ user-facing change? No. This is a new feature in Apache Spark 3.3. ### How was this patch tested? Pass the CIs with the newly added test cases.

willb changed the title ~~Fix (some of the) warnings in the test suite~~Fix postfixOps warnings in the test suiteJul 8, 2014

Ensure language.postfixOps is in scope where used
c6c0b7a
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.

willb closed this Sep 12, 2014

andrewor14 mentioned this pull request Sep 12, 2014
SPARK-2482: Resolve sbt warnings during build #1330
Closed

Uh oh!

Conversation

willb commented Jul 7, 2014

Uh oh!

srowen commented Jul 7, 2014

Uh oh!

AmplabJenkins commented Jul 7, 2014

Uh oh!

AmplabJenkins commented Jul 7, 2014

Uh oh!

willb commented Jul 7, 2014

Uh oh!

AmplabJenkins commented Jul 7, 2014

Uh oh!

AmplabJenkins commented Jul 7, 2014

Uh oh!

rxin commented Jul 8, 2014

Uh oh!

AmplabJenkins commented Jul 8, 2014

Uh oh!

AmplabJenkins commented Jul 8, 2014

Uh oh!

willb commented Jul 8, 2014

Uh oh!

AmplabJenkins commented Jul 8, 2014

Uh oh!

AmplabJenkins commented Jul 8, 2014

Uh oh!

SparkQA commented Aug 4, 2014

Uh oh!

witgo commented Aug 4, 2014

Uh oh!

SparkQA commented Aug 4, 2014

Uh oh!

willb commented Aug 6, 2014

Uh oh!

andrewor14 commented Aug 25, 2014

Uh oh!

jkbradley commented Sep 10, 2014

Uh oh!

willb commented Sep 12, 2014

Uh oh!

andrewor14 commented Sep 12, 2014

Uh oh!

willb commented Sep 12, 2014

Uh oh!

andrewor14 commented Sep 12, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants