Skip to content

tests: restore direct mock/includes and deps for admin_filter and generic_proxy upstream tests#140

Draft
Copilot wants to merge 3 commits into
test-mock-includesfrom
copilot/fix-narrow-test-mock-includes
Draft

tests: restore direct mock/includes and deps for admin_filter and generic_proxy upstream tests#140
Copilot wants to merge 3 commits into
test-mock-includesfrom
copilot/fix-narrow-test-mock-includes

Conversation

Copy link
Copy Markdown

Copilot AI commented May 12, 2026

Follow-up to mock-include narrowing: two tests were implicitly relying on transitive server-mock includes/deps and now fail to compile after narrowing. This updates those tests to include and depend on the symbols they directly use.

  • test/server/admin/admin_filter_test.cc / test/server/admin/BUILD

    • Add direct include for Stats::IsolatedStoreImpl:
      • source/common/stats/isolated_store_impl.h
    • Add matching Bazel dep:
      • //source/common/stats:isolated_store_lib
  • test/extensions/filters/network/generic_proxy/router/upstream_test.cc / .../router/BUILD

    • Add direct mock headers used by the test:
      • test/mocks/network/connection.h
      • test/mocks/upstream/thread_local_cluster.h
    • Add missing gMock using declarations for direct symbol usage:
      • using testing::_;
      • using testing::ByMove;
    • Add narrow Bazel deps:
      • //test/mocks/network:connection_mocks
      • //test/mocks/upstream:thread_local_cluster_mocks
#include "test/mocks/network/connection.h"
#include "test/mocks/upstream/thread_local_cluster.h"

using testing::_;
using testing::ByMove;

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • dl.google.com
    • Triggering command: /build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/bin/java bazel(envoy) --add-opens=java.base/java.lang=ALL-UNNAMED -Xverify:none -Djava.util.logging.config.file=/build/bazel_root/base/javalog.properties -Dcom.google.devtools.build.lib.util.LogHandlerQuerier.class=com.google.devtools.build.lib.util.SimpleLogHandler$HandlerQuerier -XX:-MaxFDLimit -Djava.library.path=/build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/lib:/build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/embedded_tools/jdk/lib/server:/build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/ -Dfile.encoding=ISO-8859-1 -Duser.country= -Duser.language= -Duser.variant= -Xmx3g -DBAZEL_TRACK_SOURCE_DIRECTORIES=1 -Djavax.net.ssl.trustStore=/tmp/custom-cacerts -Djavax.net.ssl.trustStorePassword=changeit -jar /build/bazel_root/install/fb2a7f6d344d2f4e335882534df59296/A-server.jar --max_idle_secs=10800 --noshutdown_on_low_sys_mem --connect_timeout_secs=30 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

Context

This is a follow-up to PR envoyproxy#45037 ("[WIP] tests: Narrow test mock includes more generally"), branch test-mock-includes in this repo (phlax/envoy). The tools/code/narrow_test_mocks tool stripped includes/deps from two test files that were silently relying on transitive includes from server mock headers. CI is now failing on those two files. We need to fix the symptoms (add the missing direct includes + BUILD deps) — do not change narrow_test_mocks.py itself in this PR.

Failure 1: test/server/admin/admin_filter_test.cc

Compiler errors:

test/server/admin/admin_filter_test.cc:28:18: error: no member named 'MockStreamDecoderFilterCallbacks' in namespace 'Envoy::Http'
   28 |   NiceMock<Http::MockStreamDecoderFilterCallbacks> callbacks_;
test/server/admin/admin_filter_test.cc:29:9: error: no type named 'TestRequestHeaderMapImpl' in namespace 'Envoy::Http'; did you mean 'RequestHeaderMapImpl'?
   29 |   Http::TestRequestHeaderMapImpl request_headers_;
test/server/admin/admin_filter_test.cc:21:37: error: use of undeclared identifier '_'; did you mean 'testing::_'?
   21 |     EXPECT_CALL(admin_, makeRequest(_)).WillOnce(Return(ByMove(adminHandlerCallback())));

Also note line 26 references Stats::IsolatedStoreImpl (currently builds because of transitive includes — make it direct anyway since we're touching the file).

The file currently only includes:

#include "source/server/admin/admin.h"
#include "source/server/admin/admin_filter.h"
#include "test/mocks/server/admin.h"
#include "test/test_common/environment.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"

Fix

Add direct includes to test/server/admin/admin_filter_test.cc:

#include "source/common/stats/isolated_store_impl.h"

#include "test/mocks/http/stream_decoder_filter_callbacks.h"
#include "test/test_common/utility.h"

(test/test_common/utility.h defines Http::TestRequestHeaderMapImpl and pulls in testing::_ via gmock includes plus the using testing::_ line that's already implicit in many test headers — actually be safe and also add using testing::_; to the existing using block at the top of the file alongside the existing using testing::ByMove; using testing::InSequence; using testing::NiceMock; using testing::Return;.)

Update test/server/admin/BUILD for the admin_filter_test target — currently:

envoy_cc_test(
    name = "admin_filter_test",
    srcs = envoy_select_admin_functionality(["admin_filter_test.cc"]),
    rbe_pool = "6gig",
    deps = [
        "//source/server/admin:admin_lib",
        "//test/mocks/server:instance_mocks",
        "//test/test_common:environment_lib",
    ],
)

Change to:

envoy_cc_test(
    name = "admin_filter_test",
    srcs = envoy_select_admin_functionality(["admin_filter_test.cc"]),
    rbe_pool = "6gig",
    deps = [
        "//source/common/stats:stats_lib",
        "//source/server/admin:admin_lib",
        "//test/mocks/http:stream_decoder_filter_callbacks_mocks",
        "//test/mocks/server:admin_mocks",
        "//test/test_common:environment_lib",
        "//test/test_common:utility_lib",
    ],
)

Verify the exact target name for the stream decoder filter callbacks mock by inspecting test/mocks/http/BUILD — it may be :stream_decoder_filter_callbacks_mocks or part of :http_mocks. If the dedicated narrow target doesn't exist, use //test/mocks/http:http_mocks and #include "test/mocks/http/mocks.h" instead. Also check the correct stats target — //source/common/stats:isolated_store_lib may be more appropriate than :stats_lib. Pick whichever exists and exposes Stats::IsolatedStoreImpl.

:instance_mocks is no longer needed (file only uses MockAdmin), so drop it in favour of the narrower :admin_mocks.

Failure 2: test/extensions/filters/network/generic_proxy/router/upstream_test.cc

Compiler errors include:

upstream_test.cc:129:22: error: no member named 'MockThreadLocalCluster' in namespace 'Envoy::Upstream'
upstream_test.cc:144:28: error: cannot initialize object parameter of type '...UpstreamTest' with an expression of type '...UpstreamTest_BoundGenericUpstreamWillBeReusedForSameConnection_Test'

(The second class of errors — the "cannot initialize object parameter" ones for every EXPECT_CALL and createBoundGenericUpstream reference — are downstream symptoms of _ being undeclared inside the macro expansion; once _ is fixed they go away.)

The file currently includes:

#include <memory>

#include "source/common/tracing/common_values.h"
#include "source/extensions/filters/network/generic_proxy/router/router.h"

#include "test/extensions/filters/network/generic_proxy/fake_codec.h"
#include "test/extensions/filters/network/generic_proxy/mocks/codec.h"
#include "test/extensions/filters/network/generic_proxy/mocks/filter.h"
#include "test/extensions/filters/network/generic_proxy/mocks/route.h"
#include "test/test_...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

phlax and others added 2 commits May 12, 2026 17:11
Signed-off-by: Ryan Northey <ryan@synca.io>
…eneric_proxy upstream tests

Agent-Logs-Url: https://github.com/phlax/envoy/sessions/8c4d59cf-cd3e-4148-9d65-878bc5dfef70

Co-authored-by: phlax <454682+phlax@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix missing includes and dependencies in admin_filter_test tests: restore direct mock/includes and deps for admin_filter and generic_proxy upstream tests May 12, 2026
Copilot AI requested a review from phlax May 12, 2026 16:29
Copilot finished work on behalf of phlax May 12, 2026 16:29
@phlax phlax force-pushed the test-mock-includes branch from a04c54d to ec053f5 Compare May 12, 2026 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants