Fix e2e test #246
Fix e2e test #246
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
approved
✅ Deploy Preview for gateway-api-inference-extension ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| var got []string | ||
| for m := range actual { | ||
| got = append(got, m) | ||
| } |
There was a problem hiding this comment.
L101 is the only required change to fix the e2e test, see: #250. What are these changes fixing? IMHO the existing logic is easier to read and understand.
There was a problem hiding this comment.
+1 it seems we don't use the map here
There was a problem hiding this comment.
We need this because we can't guarantee that the traffic splitting logic will select adapter 1 then adapter 2; it could do [adapter 1, adapter 1, adapter 2, adapter 2], and so you need a map to dedup, otherwise the slice in the old code will look like [adapter1, adapter1, adapter 2] vs the expected [adapter1, adapter2]
There was a problem hiding this comment.
Ah, the int threw me off. So we are treating this like a set and just verifying that we are hitting each distinct targetModel. Yeah, agreed, this prevents flakes.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahg-g, danehans The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
@danehans