Copilot commented on code in PR #13284:
URL: https://github.com/apache/apisix/pull/13284#discussion_r3129745908
##########
t/stream-plugin/traffic-split.t:
##########
@@ -244,3 +244,64 @@ hello world from port 1995
--- stream_enable
--- error_log
Connection refused
+
+
+
+=== TEST 5: set stream route with traffic-split using route_id in match
condition
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/stream_routes/3',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "traffic-split": {
+ "rules": [{
+ "match": [
+ {
+ "vars": [["route_id", "==", "3"]]
+ }
+ ],
+ "weighted_upstreams": [
+ {
+ "upstream": {
+ "name": "upstream_A",
+ "type": "roundrobin",
+ "nodes": {
+ "127.0.0.1:1995": 1
+ }
+ },
+ "weight": 1
+ }
+ ]
+ }]
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1997": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 6: route_id match condition directs traffic to plugin upstream
+--- request
+GET /hit
+--- response_body
+hello world from port 1995
+--- stream_enable
Review Comment:
TEST 6 asserts a single /hit request always returns port 1995. If an earlier
catch-all stream route (eg stream_routes/1) is matched instead of route 3,
traffic-split can select a different upstream (eg 1996) and cause intermittent
failures. After ensuring route 3 is the active match, consider also making the
assertion robust to the plugin’s selection logic (or configure the competing
routes so they can’t match).
##########
t/stream-plugin/traffic-split.t:
##########
@@ -244,3 +244,64 @@ hello world from port 1995
--- stream_enable
--- error_log
Connection refused
+
+
+
+=== TEST 5: set stream route with traffic-split using route_id in match
condition
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/stream_routes/3',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "traffic-split": {
+ "rules": [{
+ "match": [
+ {
+ "vars": [["route_id", "==", "3"]]
+ }
+ ],
+ "weighted_upstreams": [
+ {
+ "upstream": {
+ "name": "upstream_A",
+ "type": "roundrobin",
+ "nodes": {
+ "127.0.0.1:1995": 1
+ }
+ },
+ "weight": 1
+ }
+ ]
+ }]
+ }
Review Comment:
The new test route (stream_routes/3) is a catch-all route (no
sni/server_addr/server_port/remote_addr constraints). Since this test file
already created stream_routes/1 and /2 earlier (also catch-all), the stream
router will match the first route in its internal order and may never reach
route 3. This makes TEST 6 flaky and may not actually validate `route_id`
matching. Consider deleting or narrowing earlier routes before creating route 3
(or explicitly removing routes 1/2 in a cleanup step) so route 3 is guaranteed
to be the matched route for the stream connection.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
