Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Local Testing] SNOW-1063738 Run all tests against Local Testing by default Part 1 #1483

Merged
merged 12 commits into from
May 9, 2024

Conversation

sfc-gh-stan
Copy link
Collaborator

This PR is a test only change that aims to run all tests against local testing by default. The changes are to

  • xfail tests that don't apply to Local Testing, i.e. SQL feature tests like test_bind_variable.py
  • skipif tests that need an additional feature support/a bug fix/some refactoring to be enabled in Local Testing.
  • refactor some tests that use session.sql unnecessarily to run it against Local Testing

@sfc-gh-stan sfc-gh-stan requested a review from a team as a code owner May 1, 2024 20:29
@sfc-gh-stan sfc-gh-stan requested a review from sfc-gh-yuwang May 1, 2024 20:29
@sfc-gh-stan sfc-gh-stan added the NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md label May 1, 2024
@sfc-gh-stan sfc-gh-stan added the local testing Local Testing issues/PRs label May 3, 2024
@sfc-gh-stan sfc-gh-stan force-pushed the stan-flip-local-testing-test-setup branch from 069da28 to e7f82df Compare May 3, 2024 23:06
Comment on lines 109 to 110
if not local_testing_mode:
test_files = TestFiles(resources_path)
Utils.create_stage(session, tmp_stage_name, is_temporary=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

btw there is one change I made in my PR to turn create_stage/drop_stage into no-op for local testing, so that we can reduce some if-else check in the test code

    @staticmethod
    def create_stage(session: "Session", name: str, is_temporary: bool = True):
        if isinstance(session._conn, MockServerConnection):
            # no-op in local testing
            return
        session._run_query(
            f"create or replace {'temporary' if is_temporary else ''} stage {quote_name(name)}"
        )

    @staticmethod
    def drop_stage(session: "Session", name: str):
        if isinstance(session._conn, MockServerConnection):
            # no-op in local testing
            return
        session._run_query(f"drop stage if exists {quote_name(name)}")

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good, if I merge later I will remove this if block.

Copy link
Contributor

Choose a reason for hiding this comment

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

minor: ditto on removing if not local_testing_mode: check

@sfc-gh-stan sfc-gh-stan requested a review from a team as a code owner May 8, 2024 21:51
@sfc-gh-stan sfc-gh-stan force-pushed the stan-flip-local-testing-test-setup branch from e116ac1 to 14d5af4 Compare May 8, 2024 21:56
Copy link
Contributor

@sfc-gh-jrose sfc-gh-jrose left a comment

Choose a reason for hiding this comment

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

I marked some of them, but there seems to be a lot of TODOs without ticket numbers.

tests/integ/test_column_names.py Outdated Show resolved Hide resolved
tests/integ/test_column_names.py Outdated Show resolved Hide resolved
tests/integ/test_column_names.py Outdated Show resolved Hide resolved
tests/integ/test_column_names.py Outdated Show resolved Hide resolved
tests/integ/test_column_names.py Outdated Show resolved Hide resolved
tests/integ/test_dataframe.py Outdated Show resolved Hide resolved
tests/integ/test_dataframe.py Outdated Show resolved Hide resolved
tests/integ/test_dataframe.py Outdated Show resolved Hide resolved
tests/integ/test_dataframe.py Show resolved Hide resolved
tests/integ/test_dataframe.py Outdated Show resolved Hide resolved
@sfc-gh-stan
Copy link
Collaborator Author

I marked some of them, but there seems to be a lot of TODOs without ticket numbers.

This is a good point, I'll remove the TODOS for items that are not immediate TODOS, i.e. unsupported feature planned to be worked on later. I'll keep the TODOs for bug fixes and create tickets.

@sfc-gh-stan sfc-gh-stan merged commit 2efaa34 into main May 9, 2024
25 checks passed
@sfc-gh-stan sfc-gh-stan deleted the stan-flip-local-testing-test-setup branch May 9, 2024 22:34
@github-actions github-actions bot locked and limited conversation to collaborators May 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
local testing Local Testing issues/PRs NO-CHANGELOG-UPDATES This pull request does not need to update CHANGELOG.md
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants