I would expect a test with multiple external metrics, where the HPA scales based on the maximum value among them. In this test you can set one metric to the value of 0 and another has a higher value then the HPA should scale according to the maximum metric.

Thank you for the review and recommendation @omerap12 , I've updated the multi-metric test and added a new test case that uses the average value metric type.
The existing multi-metric test verifies that setting one metric to 0 while another stays high prevents scale-down, as the HPA selects the maximum across all metrics.
The new test extends this validation: after scaling to max, dropping the dominant metric causes the HPA to scale down only to the level required by the remaining metric, not to minimum. This demonstrates that MAX selection holds at intermediate replica counts. The average value metric type enables precise control over these intermediate replica counts during testing.

Why do we need WithSlow?

I initially added the slow labeling while investigating test failures in CI, but since these tests only take 2-4 minutes, I've now removed it. Thanks for catching that!

This can be simplified by just creating one metric with 0 value and another one with value > 0 and just validate that the HPA is scaling the workload up to max replicas.
I don't see the point by waiting to HPA to scale up to max replicas and then wait again to verify it does not scales down.

Updated accordingly. You're right, that's much simpler.

This is not accurate, HPA should be scaled to max replicas in this test

This is unnecessary.

Why? if we are using AverageValue we can use AverageValue with different values that will cause the HPA to scale out but not to max replicas.

Actually, I'm thinking of adding a dedicated test for the average value metric type. The idea is to make it distinct from the other tests so we can specifically validate how HPA handles intermediate replica counts when metrics change.

Let's use different value so it be something similar then comparing 0 to other value

I am not sure that this logic makes sense, how about:

func CleanupExternalMetricsServer(ctx context.Context, c clientset.Interface, ns, name string) {
	ginkgo.By(fmt.Sprintf("Cleaning up external metrics server %s", name))

	// Delete the APIService
	config, err := framework.LoadConfig()
	if err != nil {
		framework.Failf("%s", err)
	} 
	aggClient, err := aggregatorclient.NewForConfig(config)
	if err != nil {
		framework.Failf("could not create aggregator client: %v", err)
	}
	
	err = aggClient.ApiregistrationV1().APIServices().Delete(ctx, "v1beta1.external.metrics.k8s.io", metav1.DeleteOptions{})
	if err != nil && !apierrors.IsNotFound(err) {
		framework.Logf("Error deleting APIService: %v", err)
	}
	
	// Delete the deployment
	err = c.AppsV1().Deployments(ns).Delete(ctx, name, metav1.DeleteOptions{})
	if err != nil {
		framework.Logf("Error deleting deployment: %v", err)
	}

	// Delete the service
	err = c.CoreV1().Services(ns).Delete(ctx, name, metav1.DeleteOptions{})
	if err != nil {
		framework.Logf("Error deleting service: %v", err)
	}
}

Good point thank you @omerap12 . I updated to fail fast instead of silently skipping the APIService deletion.

TBH I am not sure this is needed.. resources should be cleaned using the DeferCleanup function. And I think that if a test fails you will get a new cluster from scratch so no "old resources" should be lived there.
@adrianmoisey?

Actually, I added it to fix an "already exists" error in CI. Tests were previously running in parallel. When I switched them to serial, the API service issues disappeared. However, I kept the fix in place as a safety measure in case we encounter the same issue again when adding new tests that run in parallel.
I agree we can remove it if all tests will run serially going forward

This is likely happening because ApiregistrationV1 is being created in every test. When the tests run in parallel, the resources may not be cleaned up before other tests finish, causing conflicts. Ideally, we should support parallel execution, but for now it’s fine to keep the tests running serially and improve this later.