add clean_up method for store_test_failures tests to stop hanging art… #803
Conversation
…ifacts in bigquery
| class TestBigQueryStoreTestFailures(StoreTestFailuresBase): | ||
| def test_store_and_assert(self, project): | ||
| @pytest.fixture(scope="function") | ||
| def clean_up(self, project): |
There was a problem hiding this comment.
This looks like a teardown method. In my experience, such a method tends to get called teardown_method ("method" not because this is a method, but because the scope is "function") and runs automatically.
With that in mind, would you mind if we rename it, add autouse=True in the fixture decorator, and remove clean_up from the signature of test_store_and_assert? It would all behave the same (unless StoreTestFailuresBase has additional tests), but it would follow conventions that I've seen.
There was a problem hiding this comment.
I think we historically in other tests call this teardown method clean_up as it differs from the default one all functional tests run that only drop a default schema name. if we'd like to pivot to calling these make sure all schemas are dropped methods to teardown might be a little project across all adapters and core. would just have to make sure we don't cause issue with normal run (can look into it as part of support rotation).
I can retest the autouse=True field but if I recall that for some reason didn't work when I was testing this initially we have other examples of it being a function scope and passed into the tests to trigger.
…mcknight/fix-test-store-test
…-labs/dbt-bigquery into mcknight/fix-test-store-test
| class TestBigQueryStoreTestFailures(StoreTestFailuresBase): | ||
| @pytest.fixture(autouse=True) | ||
| def teardown_method(self, project): |
There was a problem hiding this comment.
Should this be something we go change in all tests that have a clean_up method instead?
There was a problem hiding this comment.
I don't think we need to actively change it, but if we're adding new items or updating something, then I'd change it as part of that. Also, you still need scope="function", so it would be @pytest.fixture(scope="function", autouse=True)
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.5.latest 1.5.latest
# Navigate to the new working tree
cd .worktrees/backport-1.5.latest
# Create a new branch
git switch --create backport-803-to-1.5.latest
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 2fb56817037ef3f4ad8723765ed2db862edfb897
# Push it to GitHub
git push --set-upstream origin backport-803-to-1.5.latest
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.5.latestThen, create a pull request where the |
…ifacts in bigquery
resolves # hanging artifacts left during test runs due to change in schema name not caught by default teardown method
Description
would look to what branches we need to backport
Checklist
changie newto create a changelog entry