test: add unit tests for Lua HTTP filter translation within the xDS translator. #7988
Conversation
…ranslator. Signed-off-by: liuhy <liuhongyu@apache.org>
✅ Deploy Preview for cerulean-figolla-1f9435 canceled.
|
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive unit tests for the Lua HTTP filter translation functionality within the xDS translator. The tests ensure that Lua filters are properly configured and integrated into Envoy's HTTP Connection Manager.
Changes:
- Added 729 lines of unit tests for Lua filter translation
- Includes tests for HCM patching, filter building, route patching, and integration scenarios
- Covers edge cases like nil inputs, empty filters, duplicates, and validation
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| name: "empty lua code", | ||
| lua: ir.Lua{ | ||
| Name: "empty-lua", | ||
| Code: ptr.To(""), | ||
| }, | ||
| wantErr: false, | ||
| validate: func(t *testing.T, filter *hcmv3.HttpFilter) { | ||
| luaConfig := &luafilterv3.Lua{} | ||
| err := filter.GetTypedConfig().UnmarshalTo(luaConfig) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, "", luaConfig.DefaultSourceCode.GetInlineString()) | ||
| }, | ||
| }, |
There was a problem hiding this comment.
The test case "empty lua code" tests a scenario where the Lua code pointer is set to an empty string. However, based on the implementation in buildHCMLuaFilter (line 71 in lua.go), the code dereferences the pointer with *lua.Code without checking if it's nil. This test should also include a case where lua.Code is nil to ensure that buildHCMLuaFilter handles or properly errors on nil Code pointers, which would be a more realistic edge case.
| // Verify the per-route config | ||
| filterConfig := route.TypedPerFilterConfig[filterName] | ||
| require.NotNil(t, filterConfig) | ||
|
|
||
| // The config should be an empty Any (filter is just enabled) | ||
| routeFilterConfig := &routev3.FilterConfig{} | ||
| err = filterConfig.UnmarshalTo(routeFilterConfig) | ||
| require.NoError(t, err) | ||
| assert.NotNil(t, routeFilterConfig.Config) |
There was a problem hiding this comment.
The integration test doesn't verify the actual behavior when the filter is enabled on a route. The test validates that the filter configuration is added to TypedPerFilterConfig, but doesn't verify that the configuration can be successfully unmarshaled or that it has the expected structure. Consider adding validation that the routev3.FilterConfig can be unmarshaled and contains the expected empty Any config.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7988 +/- ##
==========================================
+ Coverage 72.86% 72.88% +0.02%
==========================================
Files 237 237
Lines 35536 35536
==========================================
+ Hits 25894 25902 +8
+ Misses 7799 7798 -1
+ Partials 1843 1836 -7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ter test assertions. Signed-off-by: liuhy <liuhongyu@apache.org>
1 participant
What type of PR is this?
add unit tests for Lua HTTP filter translation within the xDS translator.
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Release Notes: Yes/No