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.

Sign up for GitHub

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

Jump to bottom

more fix of smoke tests #4548

Merged
merged 80 commits into from
Jan 24, 2025
Merged

more fix of smoke tests #4548

merged 80 commits into from
Jan 24, 2025

Conversation

zpoint
Copy link
Collaborator

@zpoint zpoint commented Jan 9, 2025 •
edited
Loading

  • fix smoke tests bug #4492 is a prerequisite. Review and merge that PR first, then proceed to review this PR.
  • sky serve tests fix
  • separate the serve tests based on parameters to different buildkite agent, example: separate test_skyserve_new_autoscaler_update[rolling] and test_skyserve_new_autoscaler_update[blue_green]
  • Align the buildkite command with pytest: now use args to pass the command transparently to pytest, example:
    • /somke-test --aws
    • /somke-test --managed-jobs --aws
    • /somke-test --serve
    • /somke-test --aws -k test_nonexistent_bucket

^ The last one use -k filter to only trigger test name contains: test_nonexistent_bucket, we can trigger one test with -k option

Test

PYTHONPATH=$(pwd)/tests:$PYTHONPATH \
pytest -n 0 --dist no .buildkite/test_buildkite_pipeline_generation.py
.
9 passed, 2 warnings in 140.30s (0:02:20)

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 added 28 commits December 20, 2024 17:21
@zpoint
fix release smoke tests
33bd131
@zpoint
robust backward compatibility
d787e21
@zpoint
bug fix
49beeec
@zpoint
Merge branch 'master' into dev/zeping/fix_smoke_tests
1f98e11
@zpoint
support gke mark
b8c86ec
@zpoint
update retry field
28ce35d
@zpoint
Merge branch 'master' into dev/zeping/fix_smoke_tests
1aa9e9d
@zpoint
fix
38a26bb
@zpoint
fix smoke tests issue
e117e5e
@zpoint
update command of GCP
caa0b86
@zpoint
comment for azure fix
1a878bd
@zpoint
merge master
6fd05bb
@zpoint
clear resources
30548d0
@zpoint
remove legacy_credentials
5357036
@zpoint
Merge branch 'master' into dev/zeping/fix_smoke_tests
3a51d3a
@zpoint
bug fix
9ac96b1
@zpoint
resolve PR comment
81ec3b2
@zpoint
update comment
168093a
@zpoint
text param for subprocess
d53b849
@zpoint
longer timeout
1844001
@zpoint
Merge branch 'master' into dev/zeping/fix_smoke_tests
758c31a
@zpoint
rename to is_on_gcp
3ead41b
@zpoint
merge master
a85077d
@zpoint
revert initial_delay
744163f
@zpoint
cache based on boot time
017477d
@zpoint
fix command
e701aed
@zpoint
Merge branch 'master' into dev/zeping/fix_smoke_tests
76dec93
@zpoint
test_job_pipeline
ad89aae
@zpoint
Copy link
Collaborator Author

zpoint commented Jan 9, 2025

/smoke-tests

@zpoint
Copy link
Collaborator Author

zpoint commented Jan 23, 2025

/smoke-test -k test_managed_jobs_basic

@zpoint
Copy link
Collaborator Author

zpoint commented Jan 23, 2025

/smoke-test -k test_managed_jobs_storage --azure

@zpoint
Copy link
Collaborator Author

zpoint commented Jan 23, 2025

/smoke-test

@zpoint
Copy link
Collaborator Author

zpoint commented Jan 23, 2025

/smoke-test --kubernetes

@zpoint
Copy link
Collaborator Author

zpoint commented Jan 23, 2025

/smoke-test

@Michaelvll Michaelvll added this to the v0.8.0 milestone Jan 23, 2025
Michaelvll
Michaelvll reviewed Jan 23, 2025
View reviewed changes
.buildkite/generate_pipeline.py Outdated Show resolved Hide resolved
.buildkite/generate_pipeline.py Outdated Show resolved Hide resolved
.buildkite/generate_pipeline.py Outdated Show resolved Hide resolved
tests/smoke_tests/test_mount_and_storage.py Show resolved Hide resolved
tests/smoke_tests/test_managed_job.py Outdated Show resolved Hide resolved
tests/smoke_tests/smoke_tests_utils.py Outdated Show resolved Hide resolved
tests/smoke_tests/test_managed_job.py
@@ -43,7 +42,7 @@
# when the controller being on Azure, which takes a long time for launching
# step.
@pytest.mark.managed_jobs
def test_managed_jobs(generic_cloud: str):
def test_managed_jobs_basic(generic_cloud: str):
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need this rename?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are many test_managed_jobs_xxx. Rename this function to a unique name so that it is easier to filter using -k if I want to trigger only this test.

@zpoint
Copy link
Collaborator Author

zpoint commented Jan 23, 2025

/smoke-test

@zpoint zpoint requested a review from Michaelvll January 23, 2025 10:32
Michaelvll
Michaelvll approved these changes Jan 23, 2025
View reviewed changes
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! LGTM

.buildkite/generate_pipeline.py

parsed_args = parser.parse_args(args)

# Collect chosen clouds from the flags
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add a TODO for keeping the following sync with the conftest, or do we want to reuse the one in conftest?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I can get the default cloud from conftest, but we still need to do after processing to get the final default cloud for us. Due to our resource limits—for example, we only allow AWS, Azure, GCP, and Kubernetes tests—while the clouds from conftest can include Runpod, IBM, or whatever the user provides. Still, it's good to retrieve as much as possible from conftest.

I'll leave this as a TODO since the current PR already includes many tests. I'll address this in a following PR.

@zpoint
Copy link
Collaborator Author

zpoint commented Jan 24, 2025

/smoke-test -k test_managed_jobs_storage --azure

@zpoint
Copy link
Collaborator Author

zpoint commented Jan 24, 2025

/smoke-test -k test_managed_jobs_storage --azure

@zpoint
Copy link
Collaborator Author

zpoint commented Jan 24, 2025

/quicktest-core

@zpoint
Copy link
Collaborator Author

zpoint commented Jan 24, 2025

/quicktest-core --azure

@zpoint
Copy link
Collaborator Author

zpoint commented Jan 24, 2025

/smoke-test --serve

@zpoint
Copy link
Collaborator Author

zpoint commented Jan 24, 2025

/smoke-test --managed-jobs

@zpoint
Copy link
Collaborator Author

zpoint commented Jan 24, 2025

/smoke-test --managed-jobs --gcp

@zpoint zpoint merged commit 1c94d0f into skypilot-org:master Jan 24, 2025
19 of 20 checks passed
@zpoint
Copy link
Collaborator Author

zpoint commented Jan 24, 2025 •
edited
Loading

Buildkite comment format updated to align with the local pytest format. cc @cblmemo

@cg505 cg505 mentioned this pull request Jan 24, 2025
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Michaelvll Michaelvll Michaelvll approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.8.0
Development

Successfully merging this pull request may close these issues.
2 participants
@zpoint @Michaelvll