fix: tests should not make real network calls [IDE-1269] #1013

andrewrobinsonhodges-snyk · 2025-10-08T16:50:49Z

Description

Small refactor of unit test setup to avoid making real network calls.

Integration and smoke tests are unaffected.

Checklist

Tests added and all succeed
Regenerated mocks, etc. (make generate)
Linted (make lint-fix)
README.md updated, if user-facing
License file updated, if new 3rd-party dependency is introduced

snyk-io · 2025-10-08T16:51:02Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Licenses	0	0	0	0	0 issues
✅	Code Security	0	0	0	0	0 issues
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

gemini-code-assist · 2025-10-08T16:51:04Z

Summary of Changes

Hello @andrewrobinsonhodges-snyk, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the unit test setup to strictly prevent real network calls, improving test reliability and speed. It introduces a new mock network layer, updates the UnitTest configuration, and provides detailed documentation on how to correctly set up unit tests to avoid network dependencies.

Highlights

Intent: This pull request aims to prevent unit tests from making real network calls, ensuring they are fast, reliable, secure, and can run offline. It refactors the unit test setup to use mock network access.
Key Changes: The PR introduces a new internal/testutil/mock_network.go file which defines a mockTransport that fails any real HTTP requests and a SetupEngineMockWithNetworkAccess function to configure a mock engine with this behavior. The internal/testutil/test_setup.go file's UnitTest(t) function is updated to use this mock engine setup, ensuring that unit tests are initialized with a configuration that blocks network calls. Additionally, a comprehensive internal/testutil/README.md has been added to guide developers on proper unit test setup, emphasizing the use of testutil.UnitTest(t) and di.TestInit(t) and providing best practices and troubleshooting for network-related test issues. A call to di.Init() was removed from application/server/server_test.go to align with the new mocking strategy.
Impact: Unit tests will now explicitly prevent real network calls, leading to faster execution, increased stability, and reduced reliance on external services. Developers will have clear guidelines on how to set up unit tests correctly to avoid unintended network access, and a mechanism to quickly identify and fix tests that attempt to make real HTTP calls.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request is a great step towards making the unit tests more robust and isolated by preventing real network calls. The introduction of mock_network.go and the new README.md for test utilities are excellent additions for maintainability and developer guidance.

I've identified a few areas for improvement to enhance the new testing framework's design and prevent potential issues. My main suggestions focus on:

Improving the design of the UnitTest helper to allow for mock customization.
Correcting the documentation to reflect the proper usage pattern.
Making the mock setup more robust to prevent potential panics.
Cleaning up an unused parameter in a test helper function.

Overall, these changes are very positive, and with a few adjustments, the new test utilities will be even more powerful and safer to use.

internal/testutil/README.md

gemini-code-assist · 2025-10-08T16:52:28Z

application/server/server_test.go

 	return setupCustomServer(t, c, nil)
 }

 func setupServerWithCustomDI(t *testing.T, c *config.Config, useMocks bool) (server.Local, *testsupport.JsonRPCRecorder) {


The useMocks parameter is no longer used in this function after the removal of the if !useMocks block. To improve code clarity and remove dead code, this parameter should be removed from the function signature and any call sites.

Suggested change

func setupServerWithCustomDI(t *testing.T, c *config.Config, useMocks bool) (server.Local, *testsupport.JsonRPCRecorder) {

func setupServerWithCustomDI(t *testing.T, c *config.Config) (server.Local, *testsupport.JsonRPCRecorder) {

internal/testutil/mock_network.go

bastiandoetsch · 2025-10-09T05:58:40Z

internal/testutil/test_setup.go

 	config.SetCurrentConfig(c)
 	CLIDownloadLockFileCleanUp(t, c)
-	c.Engine().GetConfiguration().Set(code_workflow.ConfigurationSastSettings, &sast_contract.SastResponse{SastEnabled: true, LocalCodeEngine: sast_contract.LocalCodeEngine{
+	engineConfig.Set(code_workflow.ConfigurationSastSettings, &sast_contract.SastResponse{SastEnabled: true, LocalCodeEngine: sast_contract.LocalCodeEngine{


Hm. If our engine is a mock, why do we need this? I would expect that we just need to set the expectation, because this Set should not do anything.

Back compatibility; most tests just retrieve and use the configuration (which is a real object, not a mock)

rrama

I am getting a lot of tests fail for me locally for the following reasons:

Unexpected call to *mocks.MockEngine.SetConfiguration([0xblah])
Unexpected call to *mocks.MockEngine.GetWorkflows([])
Unexpected call to *mocks.MockEngine.Register(...)
panic: runtime error: invalid memory address or nil pointer dereference
Condition never satisfied (for init CLI tests)
Wrong CLI distribution channel.
Getting CLI download URL not mocked or not returning what it used to.
Trying to get lessons.
Sentry events not being as expected (or trying to be sent for real).
Attempting to check CCI FF.
No obvious reason.

As such, I cannot approve until all the tests that work for me on main work for me with your changes.

rrama · 2025-10-27T11:57:49Z

internal/testutil/README.md

+
+### Dependency Injection for Tests
+
+After setting up the config, you need to initialize the dependency injection container. **Always use `di.TestInit(t)` for unit tests**:


"you may need to"*
Nit pick: Calling it a "container" seems like the wrong word.

How about:

After setting up the config, you need may to initialize the test dependencies via injection. This can be done by calling di.TestInit(t) in your unit test.

rrama · 2025-10-27T12:03:35Z

internal/testutil/README.md

+|-----------|--------------|----------|---------------|
+| Unit Test | `testutil.UnitTest(t)` | `di.TestInit(t)` | ❌ Blocked |
+| Integration Test | `testutil.IntegTest(t)` | `di.Init()` | ✅ Allowed |
+| Smoke Test | `testutil.SmokeTest(t, false)` | `di.Init()` | ✅ Allowed |


Smoke test should never mock network calls, but integration tests can do some, so maybe we need a column for "Real Network Calls" and "Mocked Network Calls".

rrama · 2025-10-27T12:05:20Z

internal/testutil/README.md

+
+**Never use `di.Init()` in unit tests** - it creates real API clients and scanners that can make network calls.
+
+### When to Use What


This section should be the first thing in the README IMO. Plus maybe add a bit about when to use what :p

internal/testutil/mock_network.go

fix: tests should not make real network calls

55a264b

andrewrobinsonhodges-snyk requested review from a team as code owners October 8, 2025 16:50

andrewrobinsonhodges-snyk temporarily deployed to Standard October 8, 2025 16:50 — with GitHub Actions Inactive

andrewrobinsonhodges-snyk temporarily deployed to MT1 October 8, 2025 16:50 — with GitHub Actions Inactive

andrewrobinsonhodges-snyk temporarily deployed to MT3 October 8, 2025 16:50 — with GitHub Actions Inactive

gemini-code-assist bot reviewed Oct 8, 2025

View reviewed changes

bastiandoetsch reviewed Oct 9, 2025

View reviewed changes

internal/testutil/mock_network.go Outdated Show resolved Hide resolved

bastiandoetsch reviewed Oct 9, 2025

View reviewed changes

andrewrobinsonhodges-snyk changed the title ~~fix: tests should not make real network calls~~ fix: tests should not make real network calls [ Oct 22, 2025

andrewrobinsonhodges-snyk changed the title ~~fix: tests should not make real network calls [~~ fix: tests should not make real network calls [IDE-1269] Oct 22, 2025

Merge branch 'main' into test/avoid-network-calls

13fc93d

andrewrobinsonhodges-snyk temporarily deployed to MT3 October 24, 2025 15:20 — with GitHub Actions Inactive

andrewrobinsonhodges-snyk temporarily deployed to MT1 October 24, 2025 15:20 — with GitHub Actions Inactive

andrewrobinsonhodges-snyk temporarily deployed to Standard October 24, 2025 15:20 — with GitHub Actions Inactive

refactor: improve test setup and address somee pr comments

7750445

andrewrobinsonhodges-snyk temporarily deployed to MT3 October 24, 2025 15:57 — with GitHub Actions Inactive

andrewrobinsonhodges-snyk temporarily deployed to MT1 October 24, 2025 15:57 — with GitHub Actions Inactive

andrewrobinsonhodges-snyk temporarily deployed to Standard October 24, 2025 15:57 — with GitHub Actions Inactive

test: fix broken test from refactor

6e74d92

andrewrobinsonhodges-snyk temporarily deployed to Standard October 24, 2025 16:09 — with GitHub Actions Inactive

andrewrobinsonhodges-snyk temporarily deployed to MT1 October 24, 2025 16:09 — with GitHub Actions Inactive

andrewrobinsonhodges-snyk temporarily deployed to MT3 October 24, 2025 16:09 — with GitHub Actions Inactive

rrama requested changes Oct 27, 2025

View reviewed changes

andrewrobinsonhodges-snyk marked this pull request as draft October 27, 2025 16:14

andrewrobinsonhodges-snyk added the ⚠️ DONT MERGE Not ready to be merged yet label Oct 27, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: tests should not make real network calls [IDE-1269] #1013

fix: tests should not make real network calls [IDE-1269] #1013

andrewrobinsonhodges-snyk commented Oct 8, 2025

Uh oh!

snyk-io bot commented Oct 8, 2025 •

edited

Loading

Uh oh!

gemini-code-assist bot commented Oct 8, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

gemini-code-assist bot Oct 8, 2025

Uh oh!

Uh oh!

Uh oh!

bastiandoetsch Oct 9, 2025

Uh oh!

andrewrobinsonhodges-snyk Oct 24, 2025

Uh oh!

rrama left a comment

Uh oh!

rrama Oct 27, 2025

Uh oh!

rrama Oct 27, 2025

Uh oh!

rrama Oct 27, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	func setupServerWithCustomDI(t testing.T, c config.Config, useMocks bool) (server.Local, *testsupport.JsonRPCRecorder) {
	func setupServerWithCustomDI(t testing.T, c config.Config) (server.Local, *testsupport.JsonRPCRecorder) {


		### Dependency Injection for Tests

		After setting up the config, you need to initialize the dependency injection container. Always use `di.TestInit(t)` for unit tests:


		Never use `di.Init()` in unit tests - it creates real API clients and scanners that can make network calls.

		### When to Use What

fix: tests should not make real network calls [IDE-1269] #1013

Are you sure you want to change the base?

fix: tests should not make real network calls [IDE-1269] #1013

Conversation

andrewrobinsonhodges-snyk commented Oct 8, 2025

Description

Checklist

Uh oh!

snyk-io bot commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

gemini-code-assist bot commented Oct 8, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

gemini-code-assist bot Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bastiandoetsch Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

andrewrobinsonhodges-snyk Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

rrama left a comment

Choose a reason for hiding this comment

Uh oh!

rrama Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

rrama Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

rrama Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

snyk-io bot commented Oct 8, 2025 •

edited

Loading