Skip to content

SNOW-543851: introduce skip label #3415

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

sfc-gh-aling
Copy link
Contributor

@sfc-gh-aling sfc-gh-aling commented May 31, 2025

  1. Which Jira issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

    Fixes SNOW-543851

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
      • If this test skips Local Testing mode, I'm requesting review from @snowflakedb/local-testing
    • I am adding new logging messages
    • I am adding a new telemetry message
    • I am adding new credentials
    • I am adding a new dependency
    • If this is a new feature/behavior, I'm adding the Local Testing parity changes.
    • I acknowledge that I have ensured my changes to be thread-safe. Follow the link for more information: Thread-safe Developer Guidelines
    • If adding any arguments to public Snowpark APIs or creating new public Snowpark APIs, I acknowledge that I have ensured my changes include AST support. Follow the link for more information: AST Support Guidelines
  3. Please describe how your code solves the related issue.

introduce 4 skip labels to allow skipping tests

  • skip-local-testing-ci: skip local test running
  • skip-ast-encoding-ci: skip ast related tests running
  • skip-snowpark-pandas-ci: skip modin related tests running
  • skip-snowpark-python-ci: skip python api related tests running

runnings are triggered by label edit/ pr/ commit

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.

This seems like a lot of boilerplate added to each of the test steps. You could skip it at the top level by doing something similar to the local testing check:

if: ${{!contains(toJSON(github.event.pull_request.requested_teams), 'local-testing')}}

That doesn't let you have as nice of a message though.

@sfc-gh-aling
Copy link
Contributor Author

sfc-gh-aling commented Jun 2, 2025

@sfc-gh-jrose skip the test is not enough.. there are required merge gates, and skipping tests can't pass required merge gate, let me check if there are other ways to simplify the code change

@sfc-gh-aling
Copy link
Contributor Author

only run required tests
can opt in to run all tests

@sfc-gh-aling sfc-gh-aling marked this pull request as draft June 3, 2025 17:49
@sfc-gh-sfan sfc-gh-sfan removed their request for review July 23, 2025 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants