AlexStocks commented on PR #3256: URL: https://github.com/apache/dubbo-go/pull/3256#issuecomment-4066764065
### Code review Found 1 issue: 1. Tests pass vacuously — all "happy path" tests (`TestNewPolarisRouterApplicationNameFromParam`, `TestNewPolarisRouterApplicationNameFromAttribute`, `TestNewPolarisRouterApplicationNameFromSubURLParam`, `TestNewPolarisRouterApplicationNameFromSubURLAttribute`) only assert `require.NoError(t, err)`. In a test environment without a Polaris server, `remotingpolaris.Check()` returns `ErrorNoOpenPolarisAbility`, causing `newPolarisRouter` to return early with `openRoute: false` and nil error — **before `applicationName` is ever stored in the struct**. These tests cannot distinguish "application name resolved correctly" from "Polaris unavailable, skipped everything". The application name resolution logic (the core of this PR) is never actually exercised. Consider either mocking `remotingpolaris.Check()` or asserting on the returned struct's fields (e.g. `currentApplication`). https://github.com/apache/dubbo-go/blob/39f150a4148e7df546cd175d511518f01ffb9d15/cluster/router/polaris/router_test.go#L53-L87 🤖 Generated with [Claude Code](https://claude.ai/code) <sub>- If this code review was useful, please react with 👍. Otherwise, react with 👎.</sub> -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
