Skip to content

Conversation

@kiszk
Copy link
Member

@kiszk kiszk commented Jul 24, 2017

What changes were proposed in this pull request?

This PR ensures to call super.afterEach() in overriding afterEach() method in DatasetCacheSuite. When we override afterEach() method in Testsuite, we have to call super.afterEach().

This is a follow-up of #18719 and SPARK-21512.

How was this patch tested?

Used the existing test suite

@cloud-fan
Copy link
Contributor

LGTM

@SparkQA
Copy link

SparkQA commented Jul 24, 2017

Test build #79899 has finished for PR 18721 at commit 0e4d057.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

retest this please

@srowen
Copy link
Member

srowen commented Jul 24, 2017

@kiszk while you're here, looks like HiveExternalCatalogBackwardCompatibilitySuite, ContainerPlacementStrategySuite, SparkSessionBuilderSuite, TextSocketStreamSuite need this fix too.

@kiszk
Copy link
Member Author

kiszk commented Jul 24, 2017

@srowen good catch. I will work for that.
Another question is whether we should do this cleanup at SharedSQLContext.afterEach().
cc @gatorsmile @cloud-fan

@SparkQA
Copy link

SparkQA commented Jul 24, 2017

Test build #79900 has finished for PR 18721 at commit 0e4d057.

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

@cloud-fan
Copy link
Contributor

yea if many tests suite need the same fix, we can put it in SharedSQLContext.afterEach()

@SparkQA
Copy link

SparkQA commented Jul 24, 2017

Test build #79903 has finished for PR 18721 at commit 2ac2798.

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

// Clear all persistent datasets after each test
spark.sharedState.cacheManager.clearCache()
} finally {
super.afterEach()
Copy link
Contributor

Choose a reason for hiding this comment

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

sorry to be nit-pick, but seems we can just do

super.afterEach()
// Clear all persistent datasets after each test
spark.sharedState.cacheManager.clearCache()

Copy link
Member Author

Choose a reason for hiding this comment

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

I see. done

@SparkQA
Copy link

SparkQA commented Jul 24, 2017

Test build #79912 has finished for PR 18721 at commit bd6c2ad.

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

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@asfgit asfgit closed this in 7f29505 Jul 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants