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

support filter flag for buildkite #4534

Conversation

zpoint
Copy link
Collaborator

@zpoint zpoint commented Jan 5, 2025

support

PYTHONPATH=$(pwd)/tests:$PYTHONPATH python .buildkite/generate_pipeline.py --filter-marks managed_jobs

So that we can configure Buildkite to call this script with this flag, supporting arbitrary pytest flags.

Now we can comment on PR:

  • /smoke-test to trigger all smoke tests.
  • Or use /smoke-test managed_jobs to trigger smoke tests marked by certain pytest mark.
  • Format: /smoke-test <mark1>,<mark2>

Tested (run the relevant ones):

  • Code formatting: bash format.sh
  • Any manual or new tests for this PR (please specify below)
  • All smoke tests: pytest tests/test_smoke.py
  • Relevant individual smoke tests: pytest tests/test_smoke.py::test_fill_in_the_name
  • Backward compatibility tests: conda deactivate; bash -i tests/backward_compatibility_tests.sh

@zpoint
Copy link
Collaborator Author

zpoint commented Jan 5, 2025

/smoke-test managed_jobs

@zpoint
Copy link
Collaborator Author

zpoint commented Jan 5, 2025

/quicktest-core

@zpoint zpoint requested a review from Michaelvll January 5, 2025 06:20
@zpoint zpoint requested a review from Michaelvll January 6, 2025 08:02
Comment on lines 81 to 82
output = subprocess.run(cmd, shell=True, capture_output=True, text=True)
matches = re.findall('Collected (.+?) with marks: \[(.*?)\]', output.stdout)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why don't we directly pass the flag to the cmd run and let pytest figure out the tests to run?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Pytest doesn't print filtered tests; it prints all tests. Besides, I need the mark, but pytest doesn't print it.
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Michaelvll if you are asking why we need to extract the test list at all, instead of just letting pytest figure it out at test time, it's described in the docstring

Copy link
Collaborator

@Michaelvll Michaelvll left a comment

Choose a reason for hiding this comment

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

Thanks @zpoint! I am good with this PR if this works : )

@zpoint
Copy link
Collaborator Author

zpoint commented Jan 9, 2025

/smoke-test managed_jobs

@zpoint
Copy link
Collaborator Author

zpoint commented Jan 9, 2025

/smoke-test managed_jobs

@zpoint
Copy link
Collaborator Author

zpoint commented Jan 9, 2025

/smoke-test managed_jobs,serve

@zpoint
Copy link
Collaborator Author

zpoint commented Jan 9, 2025

/smoke-test

@zpoint zpoint merged commit 9b1ec55 into skypilot-org:master Jan 9, 2025
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants