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

chore: isolate execution of snowflake integration tests #2278

Merged
merged 5 commits into from
Dec 19, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 55 additions & 25 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -233,43 +233,21 @@ jobs:
- name: setup GCP
uses: google-github-actions/setup-gcloud@v1

- name: snowflake setup (SnowSQL)
run: |
curl -o snowsql.bash \
https://sfc-repo.snowflakecomputing.com/snowsql/bootstrap/1.2/linux_x86_64/snowsql-1.2.24-linux_x86_64.bash
mkdir -p ~/bin
SNOWSQL_DEST=~/bin SNOWSQL_LOGIN_SHELL=~/.profile bash snowsql.bash

- name: prepare testdata
run: ./scripts/prepare-testdata.sh

- name: setup fixtures (slt)
- name: run tests (slt)
env:
GCP_SERVICE_ACCOUNT_KEY: ${{ secrets.GCP_SERVICE_ACCOUNT_JSON }}
GCP_PROJECT_ID: glaredb-artifacts
AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
AZURE_ACCESS_KEY: ${{ secrets.AZURE_ACCESS_KEY }}
AZURE_ACCOUNT: ${{ secrets.AZURE_ACCOUNT }}
SNOWFLAKE_USERNAME: ${{ secrets.SNOWFLAKE_USERNAME }}
SNOWFLAKE_PASSWORD: ${{ secrets.SNOWFLAKE_PASSWORD }}
MINIO_ACCESS_KEY: glaredb
MINIO_SECRET_KEY: glaredb_test
TEST_BUCKET: glaredb-test-bucket
run: |
# Prepare SLT (Snowflake)
export PATH="$HOME/bin:$PATH"
if ./scripts/files-changed-in-branch.sh \
"scripts/prepare-testdata.sh" \
"scripts/create-test-snowflake-db.sh" \
"testdata/sqllogictests_datasources_common/data" \
"testdata/sqllogictests_snowflake/data"
then
export SNOWFLAKE_DATABASE=$(./scripts/create-test-snowflake-db.sh)
else
export SNOWFLAKE_DATABASE=glaredb_test
fi

# Prepare SLT (BigQuery)
if ./scripts/files-changed-in-branch.sh \
"scripts/prepare-testdata.sh" \
Expand Down Expand Up @@ -317,7 +295,7 @@ jobs:
echo "-------------------------------- WITHOUT TUNNEL TEST --------------------------------"
# Run all data source tests without running tunnel tests or the basic
# SLT tests.
just slt --exclude 'sqllogictests/*' --exclude '*/tunnels/ssh'
just slt --exclude 'sqllogictests/*' --exclude '*/tunnels/ssh' --exclude 'sqllogictests_snowflake/*'

echo "-------------------------------- WITH TUNNEL TEST --------------------------------"
# SSH tests are prone to fail if we make a lot of connections at the
Expand All @@ -331,7 +309,6 @@ jobs:
just sql-logic-tests --protocol=rpc 'sqllogictests_iceberg/*'
just sql-logic-tests --protocol=rpc 'sqllogictests_native/*'
just sql-logic-tests --protocol=rpc 'sqllogictests_object_store/*'
just sql-logic-tests --protocol=rpc 'sqllogictests_snowflake/*'
just sql-logic-tests --protocol=rpc 'sqllogictests_sqlserver/*'
just sql-logic-tests --protocol=rpc --exclude '*/tunnels/ssh' 'sqllogictests_mongodb/*'
just sql-logic-tests --protocol=rpc --exclude '*/tunnels/ssh' 'sqllogictests_mysql/*'
Expand All @@ -356,3 +333,56 @@ jobs:
# Fake GCS server with a sub-directory path; run with two different folder paths to assert no conflicts arise
just slt -l gs://$TEST_BUCKET/path/to/folder/1 -o service_account_path=/tmp/fake-gcs-creds.json 'sqllogictests_native/*'
just slt -l gs://$TEST_BUCKET/path/to/folder/2 -o service_account_path=/tmp/fake-gcs-creds.json 'sqllogictests_native/*'

service-integration-tests-snowflake:
Copy link
Contributor

@universalmind303 universalmind303 Dec 19, 2023

Choose a reason for hiding this comment

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

I think we should just allow this one to fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then why even bother running the test? If you run a test that don't look at the output of and that can't fail, then it provides no value and we should just skip it entirely.

A few things:

  • this is no longer in the required builders for tests.
  • there's nothing that depends on this
  • the changes to the concurrency settings (should) mean that only one of these tasks can run at once, which will (once all PRs will remediate the underlying cause of the failure (e.g. that too many tests are running at once, according to their error message, which agrees informally with what we see.)

I think we can add this in later, but I'd like to give this a week to settle in and see if it works in practice.

if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.owner.login == 'GlareDB'
name: Snowflake Service Integration Tests (SLT::Snowflake)
runs-on: ubuntu-latest-8-cores
needs: ["service-integration-tests"]
concurrency:
group: snowflake-integration-tests
cancel-in-progress: false
steps:
- name: checkout
uses: actions/checkout@v4
- uses: extractions/setup-just@v1
- uses: actions/cache@v3
name: cache
with:
path: |
~/.cargo/
target/
!target/**/glaredb
key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}

- name: snowflake setup (SnowSQL)
run: |
curl -o snowsql.bash \
https://sfc-repo.snowflakecomputing.com/snowsql/bootstrap/1.2/linux_x86_64/snowsql-1.2.24-linux_x86_64.bash
mkdir -p ~/bin
SNOWSQL_DEST=~/bin SNOWSQL_LOGIN_SHELL=~/.profile bash snowsql.bash

- name: prepare testdata
run: ./scripts/prepare-testdata.sh

- name: run tests (slt)
env:
SNOWFLAKE_USERNAME: ${{ secrets.SNOWFLAKE_USERNAME }}
SNOWFLAKE_PASSWORD: ${{ secrets.SNOWFLAKE_PASSWORD }}
run: |
# Prepare SLT (Snowflake)
export PATH="$HOME/bin:$PATH"
if ./scripts/files-changed-in-branch.sh \
"scripts/prepare-testdata.sh" \
"scripts/create-test-snowflake-db.sh" \
"testdata/sqllogictests_datasources_common/data" \
"testdata/sqllogictests_snowflake/data"
then
export SNOWFLAKE_DATABASE=$(./scripts/create-test-snowflake-db.sh)
else
export SNOWFLAKE_DATABASE=glaredb_test
fi

just sql-logic-tests 'sqllogictests_snowflake/*'

just sql-logic-tests --protocol=rpc 'sqllogictests_snowflake/*'
Loading