presto
presto copied to clipboard
Re-enable Nessie namespace/table visibility tests
testTableDataVisibility()
and testNamespaceVisibility
are both using the same namespace names (where namespace_one
is created on the main
branch), so it's possible that those tests interfere with each other if the tests run in parallel
@beinan yes that makes perfect sense to do that. I added some cleanup after each test that deletes all branches/tags and also improved the overall isolation in the tests so that parallel runs don't interfere with each other
@beinan turns out this isn't as trivial as I thought, because the query runner with nessie is usually created at the class level, while we clean up and therefore modify the underlying references at the test level (which means we're modifying the underlying state of the query runner). @ajantha-bhat can you please do a follow-up on this PR and rewrite the test in a way so that everything is properly cleaned up and tests don't interfere with each other when they run in parallel?
@ajantha-bhat can you please do a follow-up on this PR and rewrite the test in a way so that everything is properly cleaned up and tests don't interfere with each other when they run in parallel?
Sure. I can check in my free time.
ping @ajantha-bhat!
@rohanpednekar : Thanks for the remainder. I will handle it this week.
@beinan turns out this isn't as trivial as I thought, because the query runner with nessie is usually created at the class level, while we clean up and therefore modify the underlying references at the test level (which means we're modifying the underlying state of the query runner). @ajantha-bhat can you please do a follow-up on this PR and rewrite the test in a way so that everything is properly cleaned up and tests don't interfere with each other when they run in parallel?
It appears that @Test(singleThreaded = true)
fixed the issue I was seeing, so this should be ready for a re-review @beinan
@beinan, @rohanpednekar: We have added the clean-up in this PR itself today. So, there won't any follow PR from me for the same.
@beinan could you re-review this one please?
@beinan @ChunxuTang is this something that we can merge?
@beinan any idea why a different and unrelated CI action fails randomly?
@nastra Let me rerun the tests and see whether they could pass this time.
@ChunxuTang looks like CI is passing, is this good to be merged then?