add deployment status check in test

[abrarsheikh]

I suspect that when we deploy the app config, we dont wait long enough before sending traffic, so requests could go to the wrong version

[gemini-code-assist]

Code Review

This pull request improves test stability by adding checks to wait for deployments to become healthy before proceeding with the tests. This is a good practice to avoid flakiness. My review includes suggestions to consolidate the health checks for multiple deployments into a single, more efficient call to improve code readability and performance.

[gemini-code-assist]

For improved readability and efficiency, you can combine the health checks for multiple deployments into a single wait_for_condition call. This avoids the overhead of multiple wait_for_condition calls and can be more efficient by fetching the status of all deployments at once.

A helper function could be defined for this. For example:

def check_deployments_healthy(names: List[str], app_name: str = SERVE_DEFAULT_APP_NAME):
    statuses = serve.status().applications[app_name].deployments
    return all(
        statuses.get(name) and statuses[name].status == DeploymentStatus.HEALTHY
        for name in names
    )

And then call it like this:

wait_for_condition(check_deployments_healthy, names=["A", "B"])

This helper could be defined locally within the test method or, if it's generally useful, in ray.serve._private.test_utils.

[gemini-code-assist]

Similar to my other comment, these two wait_for_condition calls can be consolidated into a single call for better readability and efficiency. This can be done using a helper function that checks the status of multiple deployments at once.

[akyang-anyscale]

I suspect that when we deploy the app config, we dont wait long enough before sending traffic, so requests could go to the wrong version

isn't check_running supposed to do that waiting for us?

[abrarsheikh]

I suspect that when we deploy the app config, we dont wait long enough before sending traffic, so requests could go to the wrong version

isn't check_running supposed to do that waiting for us?

is app running imply deployment == healthy ? I am 90% sure that is true.

but also check_running was missing from test_autoscaling_policy_enable_disable

[akyang-anyscale]

is app running imply deployment == healthy ? I am 90% sure that is true

so is it redundant then?