@W-21221947: Refactor tests to stub env vars instead of mocking the Config module

src/telemetry/init.test.ts

@@ -1,31 +1,23 @@
 import { MockInstance } from 'vitest';
 
+import { stubDefaultEnvVars } from '../testShared.js';
 import { initializeTelemetry } from './init.js';
-import { TelemetryConfig } from './types.js';
-
 const mocks = vi.hoisted(() => ({
-  mockGetConfig: vi.fn(),
   MockNoOpTelemetryProvider: vi.fn(),
 }));
 
-vi.mock('../config.js', () => ({
-  getConfig: mocks.mockGetConfig,
-}));
-
 vi.mock('./noop.js', () => ({
   NoOpTelemetryProvider: mocks.MockNoOpTelemetryProvider,
 }));
 
 describe('initializeTelemetry', () => {
-  const defaultTelemetryConfig: TelemetryConfig = {
-    provider: 'noop',
-  };
-
   let consoleErrorSpy: MockInstance;
   let consoleWarnSpy: MockInstance;
 
   beforeEach(() => {
     vi.clearAllMocks();
+    vi.unstubAllEnvs();
+    stubDefaultEnvVars();
 
     // Suppress console output
     consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
@@ -41,23 +33,21 @@ describe('initializeTelemetry', () => {
   afterEach(() => {
     consoleErrorSpy.mockRestore();
     consoleWarnSpy.mockRestore();
+    vi.unstubAllEnvs();
   });
 
   // NoOp tests
   it('returns NoOpTelemetryProvider when provider is "noop"', () => {
-    mocks.mockGetConfig.mockReturnValue({
-      telemetry: { ...defaultTelemetryConfig, provider: 'noop' },
-    });
+    vi.stubEnv('TELEMETRY_PROVIDER', 'noop');
 
     initializeTelemetry();
 
     expect(mocks.MockNoOpTelemetryProvider).toHaveBeenCalled();
   });
 
-  it('returns NoOpTelemetryProvider for unknown provider with warning', () => {
-    mocks.mockGetConfig.mockReturnValue({
-      telemetry: { ...defaultTelemetryConfig, provider: 'unknown-provider' },
-    });
+  it('returns NoOpTelemetryProvider when provider is "custom" and module path is invalid', () => {
+    vi.stubEnv('TELEMETRY_PROVIDER', 'custom');
+    vi.stubEnv('TELEMETRY_PROVIDER_CONFIG', '{"module":"./invalid-module.js"}');
 
     initializeTelemetry();
 

src/testSetup.ts

@@ -1,10 +1,6 @@
-import { testProductVersion } from './testShared.js';
+import { stubDefaultEnvVars, testProductVersion } from './testShared.js';
 
-vi.stubEnv('SERVER', 'https://my-tableau-server.com');
-vi.stubEnv('SITE_NAME', 'tc25');
-vi.stubEnv('PAT_NAME', 'sponge');
-vi.stubEnv('PAT_VALUE', 'bob');
-vi.stubEnv('TABLEAU_MCP_TEST', 'true');
+stubDefaultEnvVars();
 
 vi.mock('./server.js', async (importOriginal) => ({
   ...(await importOriginal()),

src/testShared.ts

@@ -4,3 +4,12 @@ export const testProductVersion = {
   value: '2025.3.0',
   build: '20253.25.0903.0012',
 } satisfies ProductVersion;
+
+export function stubDefaultEnvVars(): void {
+  vi.stubEnv('SERVER', 'https://my-tableau-server.com');
+  vi.stubEnv('SITE_NAME', 'tc25');
+  vi.stubEnv('PAT_NAME', 'sponge');
+  vi.stubEnv('PAT_VALUE', 'bob');
+  vi.stubEnv('TABLEAU_MCP_TEST', 'true');
+  vi.stubEnv('PRODUCT_TELEMETRY_ENABLED', 'false');
+}

src/tools/getDatasourceMetadata/getDatasourceMetadata.test.ts

@@ -2,6 +2,7 @@ import { CallToolResult } from '@modelcontextprotocol/sdk/types.js';
 import { Err, Ok } from 'ts-results-es';
 
 import { Server } from '../../server.js';
+import { stubDefaultEnvVars } from '../../testShared.js';
 import invariant from '../../utils/invariant.js';
 import { Provider } from '../../utils/provider.js';
 import { getVizqlDataServiceDisabledError } from '../getVizqlDataServiceDisabledError.js';
@@ -190,7 +191,6 @@ const mockListFieldsResponses = vi.hoisted(() => ({
 const mocks = vi.hoisted(() => ({
   mockReadMetadata: vi.fn(),
   mockGraphql: vi.fn(),
-  mockGetConfig: vi.fn(),
 }));
 
 vi.mock('../../restApiInstance.js', () => ({
@@ -206,28 +206,16 @@ vi.mock('../../restApiInstance.js', () => ({
   ),
 }));
 
-vi.mock('../../config.js', () => ({
-  getConfig: mocks.mockGetConfig,
-}));
-
 describe('getDatasourceMetadataTool', () => {
   beforeEach(() => {
     vi.clearAllMocks();
-    // Set default config for existing tests
+    vi.unstubAllEnvs();
+    stubDefaultEnvVars();
     resetResourceAccessCheckerSingleton();
-    mocks.mockGetConfig.mockReturnValue({
-      productTelemetryEndpoint: 'https://test.telemetry.example.com',
-      productTelemetryEnabled: true,
-      siteName: 'test-site',
-      server: 'https://test-server.example.com',
-      disableMetadataApiRequests: false,
-      boundedContext: {
-        projectIds: null,
-        datasourceIds: null,
-        workbookIds: null,
-        tags: null,
-      },
-    });
+  });
+
+  afterEach(() => {
+    vi.unstubAllEnvs();
   });
 
   it('should create a tool instance with correct properties', () => {
@@ -646,20 +634,7 @@ describe('getDatasourceMetadataTool', () => {
   });
 
   it('should return only readMetadata result when disableMetadataApiRequests is true and readMetadata succeeds', async () => {
-    // Configure to disable metadata API requests
-    mocks.mockGetConfig.mockReturnValue({
-      productTelemetryEndpoint: 'https://test.telemetry.example.com',
-      productTelemetryEnabled: true,
-      siteName: 'test-site',
-      server: 'https://test-server.example.com',
-      disableMetadataApiRequests: true,
-      boundedContext: {
-        projectIds: null,
-        datasourceIds: null,
-        workbookIds: null,
-        tags: null,
-      },
-    });
+    vi.stubEnv('DISABLE_METADATA_API_REQUESTS', 'true');
 
     mocks.mockReadMetadata.mockResolvedValue(new Ok(mockReadMetadataResponses.success));
     mocks.mockGraphql.mockResolvedValue(mockListFieldsResponses.success);
@@ -705,20 +680,7 @@ describe('getDatasourceMetadataTool', () => {
   });
 
   it('should return error when disableMetadataApiRequests is true and readMetadata fails', async () => {
-    // Configure to disable metadata API requests
-    mocks.mockGetConfig.mockReturnValue({
-      productTelemetryEndpoint: 'https://test.telemetry.example.com',
-      productTelemetryEnabled: true,
-      siteName: 'test-site',
-      server: 'https://test-server.example.com',
-      disableMetadataApiRequests: true,
-      boundedContext: {
-        projectIds: null,
-        datasourceIds: null,
-        workbookIds: null,
-        tags: null,
-      },
-    });
+    vi.stubEnv('DISABLE_METADATA_API_REQUESTS', 'true');
 
     const errorMessage = 'ReadMetadata API Error';
     mocks.mockReadMetadata.mockRejectedValue(new Error(errorMessage));
@@ -752,18 +714,7 @@ describe('getDatasourceMetadataTool', () => {
   });
 
   it('should return data source not allowed error when datasource is not allowed', async () => {
-    mocks.mockGetConfig.mockReturnValue({
-      productTelemetryEndpoint: 'https://test.telemetry.example.com',
-      productTelemetryEnabled: true,
-      siteName: 'test-site',
-      server: 'https://test-server.example.com',
-      boundedContext: {
-        projectIds: null,
-        datasourceIds: new Set(['some-other-datasource-luid']),
-        workbookIds: null,
-        tags: null,
-      },
-    });
+    vi.stubEnv('INCLUDE_DATASOURCE_IDS', 'some-other-datasource-luid');
 
     const result = await getToolResult();
     expect(result.isError).toBe(true);

src/tools/pulse/generateInsightBrief/generatePulseInsightBriefTool.test.ts

@@ -3,6 +3,7 @@ import { Err, Ok } from 'ts-results-es';
 
 import { PulseDisabledError } from '../../../sdks/tableau/methods/pulseMethods.js';
 import { Server } from '../../../server.js';
+import { stubDefaultEnvVars } from '../../../testShared.js';
 import invariant from '../../../utils/invariant.js';
 import { Provider } from '../../../utils/provider.js';
 import { exportedForTesting as resourceAccessCheckerExportedForTesting } from '../../resourceAccessChecker.js';
@@ -12,7 +13,6 @@ const { resetResourceAccessCheckerSingleton } = resourceAccessCheckerExportedFor
 
 const mocks = vi.hoisted(() => ({
   mockGeneratePulseInsightBrief: vi.fn(),
-  mockGetConfig: vi.fn(),
 }));
 
 vi.mock('../../../restApiInstance.js', () => ({
@@ -25,10 +25,6 @@ vi.mock('../../../restApiInstance.js', () => ({
   ),
 }));
 
-vi.mock('../../../config.js', () => ({
-  getConfig: mocks.mockGetConfig,
-}));
-
 describe('getGeneratePulseInsightBriefTool', () => {
   const briefRequest = {
     language: 'LANGUAGE_EN_US' as const,
@@ -129,19 +125,13 @@ describe('getGeneratePulseInsightBriefTool', () => {
 
   beforeEach(() => {
     vi.clearAllMocks();
+    vi.unstubAllEnvs();
+    stubDefaultEnvVars();
     resetResourceAccessCheckerSingleton();
-    mocks.mockGetConfig.mockReturnValue({
-      productTelemetryEndpoint: 'https://test.telemetry.example.com',
-      productTelemetryEnabled: true,
-      siteName: 'test-site',
-      server: 'https://test-server.example.com',
-      boundedContext: {
-        projectIds: null,
-        datasourceIds: null,
-        workbookIds: null,
-        tags: null,
-      },
-    });
+  });
+
+  afterEach(() => {
+    vi.unstubAllEnvs();
   });
 
   it('should have correct tool name', () => {
@@ -307,18 +297,7 @@ describe('getGeneratePulseInsightBriefTool', () => {
       ],
     };
 
-    mocks.mockGetConfig.mockReturnValue({
-      productTelemetryEndpoint: 'https://test.telemetry.example.com',
-      productTelemetryEnabled: true,
-      siteName: 'test-site',
-      server: 'https://test-server.example.com',
-      boundedContext: {
-        projectIds: null,
-        datasourceIds: new Set([allowedDatasourceId]),
-        workbookIds: null,
-        tags: null,
-      },
-    });
+    vi.stubEnv('INCLUDE_DATASOURCE_IDS', allowedDatasourceId);
     mocks.mockGeneratePulseInsightBrief.mockResolvedValue(new Ok(mockBriefResponse));
 
     const result = await getToolResult(twoMetricRequest);
@@ -337,18 +316,7 @@ describe('getGeneratePulseInsightBriefTool', () => {
   });
 
   it('should return an error when all metrics are filtered out', async () => {
-    mocks.mockGetConfig.mockReturnValue({
-      productTelemetryEndpoint: 'https://test.telemetry.example.com',
-      productTelemetryEnabled: true,
-      siteName: 'test-site',
-      server: 'https://test-server.example.com',
-      boundedContext: {
-        projectIds: null,
-        datasourceIds: new Set(['ALLOWED-DATASOURCE-ID']),
-        workbookIds: null,
-        tags: null,
-      },
-    });
+    vi.stubEnv('INCLUDE_DATASOURCE_IDS', 'some-other-datasource-luid');
     mocks.mockGeneratePulseInsightBrief.mockResolvedValue(new Ok(mockBriefResponse));
 
     const result = await getToolResult();

src/tools/pulse/generateMetricValueInsightBundle/generatePulseMetricValueInsightBundleTool.test.ts

@@ -4,6 +4,7 @@ import { Err, Ok } from 'ts-results-es';
 import { PulseDisabledError } from '../../../sdks/tableau/methods/pulseMethods.js';
 import { PulseInsightBundleType } from '../../../sdks/tableau/types/pulse.js';
 import { Server } from '../../../server.js';
+import { stubDefaultEnvVars } from '../../../testShared.js';
 import invariant from '../../../utils/invariant.js';
 import { Provider } from '../../../utils/provider.js';
 import { exportedForTesting as resourceAccessCheckerExportedForTesting } from '../../resourceAccessChecker.js';
@@ -12,7 +13,6 @@ import { getGeneratePulseMetricValueInsightBundleTool } from './generatePulseMet
 const { resetResourceAccessCheckerSingleton } = resourceAccessCheckerExportedForTesting;
 const mocks = vi.hoisted(() => ({
   mockGeneratePulseMetricValueInsightBundle: vi.fn(),
-  mockGetConfig: vi.fn(),
 }));
 
 vi.mock('../../../restApiInstance.js', () => ({
@@ -25,10 +25,6 @@ vi.mock('../../../restApiInstance.js', () => ({
   ),
 }));
 
-vi.mock('../../../config.js', () => ({
-  getConfig: mocks.mockGetConfig,
-}));
-
 describe('getGeneratePulseMetricValueInsightBundleTool', () => {
   const bundleRequest = {
     bundle_request: {
@@ -115,21 +111,13 @@ describe('getGeneratePulseMetricValueInsightBundleTool', () => {
 
   beforeEach(() => {
     vi.clearAllMocks();
-    // Set default config for existing tests
+    vi.unstubAllEnvs();
+    stubDefaultEnvVars();
     resetResourceAccessCheckerSingleton();
-    mocks.mockGetConfig.mockReturnValue({
-      productTelemetryEndpoint: 'https://test.telemetry.example.com',
-      productTelemetryEnabled: true,
-      siteName: 'test-site',
-      server: 'https://test-server.example.com',
-      disableMetadataApiRequests: false,
-      boundedContext: {
-        projectIds: null,
-        datasourceIds: null,
-        workbookIds: null,
-        tags: null,
-      },
-    });
+  });
+
+  afterEach(() => {
+    vi.unstubAllEnvs();
   });
 
   it('should call generatePulseMetricValueInsightBundle without bundleType and return Ok result', async () => {
@@ -229,18 +217,7 @@ describe('getGeneratePulseMetricValueInsightBundleTool', () => {
   });
 
   it('should return data source not allowed error when datasource is not allowed', async () => {
-    mocks.mockGetConfig.mockReturnValue({
-      productTelemetryEndpoint: 'https://test.telemetry.example.com',
-      productTelemetryEnabled: true,
-      siteName: 'test-site',
-      server: 'https://test-server.example.com',
-      boundedContext: {
-        projectIds: null,
-        datasourceIds: new Set(['some-other-datasource-luid']),
-        workbookIds: null,
-        tags: null,
-      },
-    });
+    vi.stubEnv('INCLUDE_DATASOURCE_IDS', 'some-other-datasource-luid');
 
     const result = await getToolResult();
     expect(result.isError).toBe(true);