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

NO-SNOW: Run tests with multithreading mode enabled by default #2539

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
70 changes: 5 additions & 65 deletions .github/workflows/precommit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -471,8 +471,8 @@ jobs:
.tox/.coverage
.tox/coverage.xml

test-snowpark-multithreading-mode:
name: Test Snowpark Multithreading py-${{ matrix.os }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}
test-snowpark-disable-multithreading-mode:
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we move disable mode test to daily test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah. can do

name: Test Snowpark Multithreading Disabled py-${{ matrix.os }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}
needs: build
runs-on: ${{ matrix.os }}
strategy:
Expand Down Expand Up @@ -509,15 +509,15 @@ jobs:
- name: Install tox
run: python -m pip install tox
- name: Run tests (excluding doctests)
run: python -m tox -e "py${PYTHON_VERSION/\./}-multithreaded-ci"
run: python -m tox -e "py${PYTHON_VERSION/\./}-notmultithreaded-ci"
env:
PYTHON_VERSION: ${{ matrix.python-version }}
cloud_provider: ${{ matrix.cloud-provider }}
PYTEST_ADDOPTS: --color=yes --tb=short
TOX_PARALLEL_NO_SPINNER: 1
shell: bash
- name: Run local tests
run: python -m tox -e "py${PYTHON_VERSION/\./}-localmultithreaded-ci"
run: python -m tox -e "py${PYTHON_VERSION/\./}-localnotmultithreaded-ci"
env:
PYTHON_VERSION: ${{ matrix.python-version }}
cloud_provider: ${{ matrix.cloud-provider }}
Expand All @@ -538,65 +538,6 @@ jobs:
.tox/coverage.xml


test-snowpandas-multithreading-mode:
name: Test Snowpandas Multithreading py-${{ matrix.os }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}
needs: build
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest-64-cores]
python-version: ["3.9"]
cloud-provider: [aws]
steps:
- name: Checkout Code
uses: actions/checkout@v4
- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python-version }}
- name: Display Python version
run: python -c "import sys; print(sys.version)"
- name: Decrypt parameters.py
shell: bash
run: .github/scripts/decrypt_parameters.sh
env:
PARAMETER_PASSWORD: ${{ secrets.PARAMETER_PASSWORD }}
CLOUD_PROVIDER: ${{ matrix.cloud-provider }}
- name: Download wheel(s)
uses: actions/download-artifact@v4
with:
name: wheel
path: dist
- name: Show wheels downloaded
run: ls -lh dist
shell: bash
- name: Upgrade setuptools, pip and wheel
run: python -m pip install -U setuptools pip wheel
- name: Install tox
run: python -m pip install tox
- name: Run Snowpark pandas API tests (excluding doctests)
run: python -m tox -e "py${PYTHON_VERSION/\./}-snowparkpandasmultithreaded-modin-ci"
env:
PYTHON_VERSION: ${{ matrix.python-version }}
cloud_provider: ${{ matrix.cloud-provider }}
PYTEST_ADDOPTS: --color=yes --tb=short
TOX_PARALLEL_NO_SPINNER: 1
shell: bash
- name: Combine coverages
run: python -m tox -e coverage --skip-missing-interpreters false
shell: bash
env:
SNOWFLAKE_IS_PYTHON_RUNTIME_TEST: 1
- uses: actions/upload-artifact@v4
with:
include-hidden-files: true
name: coverage_${{ matrix.os }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}-snowpandas-multithreading
path: |
.tox/.coverage
.tox/coverage.xml


combine-coverage:
if: ${{ success() || failure() }}
name: Combine coverage
Expand All @@ -605,8 +546,7 @@ jobs:
- test-local-testing
- test-snowpark-pandas
- test-modin-extra-without-pandas-extra
- test-snowpark-multithreading-mode
- test-snowpandas-multithreading-mode
- test-snowpark-disable-multithreading-mode
runs-on: ubuntu-latest
steps:
- name: Checkout Code
Expand Down
6 changes: 4 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ def is_excluded_frontend_file(path):
def pytest_addoption(parser):
parser.addoption("--disable_sql_simplifier", action="store_true", default=False)
parser.addoption("--disable_cte_optimization", action="store_true", default=False)
parser.addoption("--multithreading_mode", action="store_true", default=False)
parser.addoption(
"--disable_multithreading_mode", action="store_true", default=False
)
parser.addoption("--skip_sql_count_check", action="store_true", default=False)
if not any(
"--local_testing_mode" in opt.names() for opt in parser._anonymous.options
Expand Down Expand Up @@ -95,7 +97,7 @@ def cte_optimization_enabled(pytestconfig):

@pytest.fixture(scope="session", autouse=True)
def multithreading_mode_enabled(pytestconfig):
enabled = pytestconfig.getoption("multithreading_mode")
enabled = not pytestconfig.getoption("disable_multithreading_mode")
global MULTITHREADING_TEST_MODE_ENABLED
MULTITHREADING_TEST_MODE_ENABLED = enabled
return enabled
Expand Down
1 change: 0 additions & 1 deletion tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ def multithreaded_run(num_threads: int = 5) -> None:
from tests.conftest import MULTITHREADING_TEST_MODE_ENABLED

def decorator(func):
@pytest.mark.multithreaded_run
@functools.wraps(func)
def wrapper(*args, **kwargs):
if MULTITHREADING_TEST_MODE_ENABLED:
Expand Down
6 changes: 2 additions & 4 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,13 @@ commands =
notudf: {env:SNOWFLAKE_PYTEST_CMD} -m "{env:SNOWFLAKE_TEST_TYPE} and not udf" {posargs:} src/snowflake/snowpark tests
udf: {env:SNOWFLAKE_PYTEST_CMD} -m "{env:SNOWFLAKE_TEST_TYPE} or udf" {posargs:} src/snowflake/snowpark tests
notdoctest: {env:SNOWFLAKE_PYTEST_CMD} -m "{env:SNOWFLAKE_TEST_TYPE} or udf" {posargs:} tests
multithreaded: {env:SNOWFLAKE_PYTEST_CMD} --multithreading_mode -m "{env:SNOWFLAKE_TEST_TYPE} or udf" {posargs:} tests
notmultithreaded: {env:SNOWFLAKE_PYTEST_CMD} --disable_multithreading_mode -m "{env:SNOWFLAKE_TEST_TYPE} or udf" {posargs:} tests
notudfdoctest: {env:SNOWFLAKE_PYTEST_CMD} -m "{env:SNOWFLAKE_TEST_TYPE} and not udf" {posargs:} tests
local: {env:SNOWFLAKE_PYTEST_CMD} --local_testing_mode -m "integ or unit or mock" {posargs:} tests
localmultithreaded: {env:SNOWFLAKE_PYTEST_CMD} --multithreading_mode --local_testing_mode -m "integ or unit or mock" {posargs:} tests
localnotmultithreaded: {env:SNOWFLAKE_PYTEST_CMD} --disable_multithreading_mode --local_testing_mode -m "integ or unit or mock" {posargs:} tests
dailynotdoctest: {env:SNOWFLAKE_PYTEST_DAILY_CMD} -m "{env:SNOWFLAKE_TEST_TYPE} or udf" {posargs:} tests
# Snowpark pandas commands:
snowparkpandasnotdoctest: {env:MODIN_PYTEST_CMD} --durations=20 -m "{env:SNOWFLAKE_TEST_TYPE}" {posargs:} {env:SNOW_1314507_WORKAROUND_RERUN_FLAGS} tests/unit/modin tests/integ/modin tests/integ/test_df_to_snowpark_pandas.py
snowparkpandasmultithreaded: {env:MODIN_PYTEST_CMD} --multithreading_mode --skip_sql_count_check --durations=20 -m "{env:SNOWFLAKE_TEST_TYPE} and multithreaded_run" {posargs:} {env:SNOW_1314507_WORKAROUND_RERUN_FLAGS} tests/integ/modin tests/integ/test_df_to_snowpark_pandas.py
# This one only run doctest but we still need to include the tests folder to let tests/conftest.py to mark the doctest files for us
snowparkpandasdoctest: {env:MODIN_PYTEST_CMD} --durations=20 -m "{env:SNOWFLAKE_TEST_TYPE}" {posargs:} src/snowflake/snowpark/modin/ tests/unit/modin
# This one is used by daily_modin_precommit.yml
Expand Down Expand Up @@ -213,7 +212,6 @@ markers =
# Other markers
timeout: tests that need a timeout time
modin_sp_precommit: modin precommit tests run in sproc
multithreaded_run: tests that need to run in multithreaded mode
addopts = --doctest-modules --timeout=1200

[flake8]
Expand Down
Loading