Skip to content

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Dec 13, 2019

What changes were proposed in this pull request?

In the PR, I propose to move all tests that use deprecated Spark APIs to separate test classes, and add the annotation:

@deprecated("This test suite will be removed.", "3.0.0")

The annotation suppress warnings from already deprecated methods and classes.

Why are the changes needed?

The warnings about deprecated Spark APIs in tests does not indicate any issues because the tests use such APIs intentionally. Eliminating the warnings allows to highlight other warnings that could show real problems.

Does this PR introduce any user-facing change?

No

How was this patch tested?

By existing test suites and by

  • DeprecatedAvroFunctionsSuite
  • DeprecatedDateFunctionsSuite
  • DeprecatedDatasetAggregatorSuite
  • DeprecatedStreamingAggregationSuite
  • DeprecatedWholeStageCodegenSuite

@SparkQA
Copy link

SparkQA commented Dec 14, 2019

Test build #115317 has finished for PR 26885 at commit eb67257.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we still need to test the deprecated methods? while they're still around they need to work.

@MaxGekk
Copy link
Member Author

MaxGekk commented Dec 15, 2019

Don't we still need to test the deprecated methods? while they're still around they need to work.

I agree that all deprecated methods need to work. I moved those tests to special test suites with the @deprecated annotation. It means the test should be executed, and should not produce any deprecation warnings.

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK so there's no net change in tests? this seems like a good approach if so.

@MaxGekk
Copy link
Member Author

MaxGekk commented Dec 15, 2019

OK so there's no net change in tests?

Right

@srowen
Copy link
Member

srowen commented Dec 17, 2019

Merged to master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants