From 5facad7b6d3a3889c35a94b64cdccf6d648fa5f9 Mon Sep 17 00:00:00 2001 From: "courtmanr@gmail.com" Date: Thu, 8 May 2025 16:52:36 +0100 Subject: [PATCH] Fix: Correct syntax error and update tests --- server/tests/apiClients.test.js | 142 ++++++++++++++++++++++++++++++- server/tests/config.test.js | 27 ++++++ server/tests/dataFetcher.test.js | 127 ++++++++++++++++++++++++++- server/tests/pbsUtils.test.js | 19 ++++- 4 files changed, 308 insertions(+), 7 deletions(-) diff --git a/server/tests/apiClients.test.js b/server/tests/apiClients.test.js index aa03908b3..868e10f40 100644 --- a/server/tests/apiClients.test.js +++ b/server/tests/apiClients.test.js @@ -26,7 +26,7 @@ jest.mock('axios-retry', () => { }; }); -const { initializeApiClients } = require('../apiClients'); +const { initializeApiClients, createApiClientInstance } = require('../apiClients'); const { loadConfiguration } = require('../configLoader'); const axios = require('axios'); // <-- Get the mocked axios const axiosRetry = require('axios-retry').default; // <-- Get the mocked default export @@ -627,6 +627,77 @@ describe('API Client Helper Functions', () => { jest.clearAllMocks(); }); + // --- Tests for createApiClientInstance --- + describe('createApiClientInstance', () => { + const { createApiClientInstance } = require('../apiClients'); + const axios = require('axios'); // Mocked axios + const axiosRetry = require('axios-retry').default; // Mocked axiosRetry + + beforeEach(() => { + // Reset axios.create and axiosRetry mocks + axios.create.mockClear(); + axiosRetry.mockClear(); + // Reconfigure axios.create to return a mock instance with spied interceptors + axios.create.mockImplementation(() => ({ + get: jest.fn(), + interceptors: { + request: { use: jest.fn() }, + response: { use: jest.fn() } + } + })); + }); + + test('should create an instance with provided baseURL and httpsAgent config', () => { + const baseURL = 'https://test.com/api'; + const allowSelfSignedCerts = true; + createApiClientInstance(baseURL, allowSelfSignedCerts); + + expect(axios.create).toHaveBeenCalledTimes(1); + expect(axios.create).toHaveBeenCalledWith(expect.objectContaining({ + baseURL: baseURL, + httpsAgent: expect.objectContaining({ + options: expect.objectContaining({ rejectUnauthorized: false }) + }), + headers: { 'Content-Type': 'application/json' } + })); + }); + + test('should call request.use when authInterceptor is provided', () => { + const mockInterceptor = jest.fn(); + const apiClient = createApiClientInstance('https://test.com', false, mockInterceptor, null); // Pass null for retryConfig + + expect(apiClient.interceptors.request.use).toHaveBeenCalledTimes(1); + expect(apiClient.interceptors.request.use).toHaveBeenCalledWith(mockInterceptor); + }); + + test('should NOT call request.use when authInterceptor is NOT provided', () => { + const apiClient = createApiClientInstance('https://test.com', false, null, null); // Pass null for both + + expect(apiClient.interceptors.request.use).not.toHaveBeenCalled(); + }); + + test('should call axiosRetry when retryConfig is provided', () => { + const mockRetryConfig = { retries: 5, retryDelayLogger: jest.fn(), retryConditionChecker: jest.fn() }; + const apiClient = createApiClientInstance('https://test.com', false, null, mockRetryConfig); + + expect(axiosRetry).toHaveBeenCalledTimes(1); + expect(axiosRetry).toHaveBeenCalledWith(apiClient, { + retries: mockRetryConfig.retries, + retryDelay: mockRetryConfig.retryDelayLogger, // Now correctly accesses the logger + retryCondition: mockRetryConfig.retryConditionChecker, // Now correctly accesses the checker + }); + }); + + test('should NOT call axiosRetry when retryConfig is NOT provided', () => { + createApiClientInstance('https://test.com', false, null, null); // Pass null for both + + expect(axiosRetry).not.toHaveBeenCalled(); + }); + + }); + + // --- createPveAuthInterceptor Tests --- + // --- createPveAuthInterceptor Tests --- describe('createPveAuthInterceptor', () => { const { createPveAuthInterceptor } = require('../apiClients'); @@ -681,6 +752,75 @@ describe('API Client Helper Functions', () => { // Note: Add test for missing creds if validation doesn't happen before calling this }); + // --- Tests for createApiClientInstance --- + describe('createApiClientInstance', () => { + const { createApiClientInstance } = require('../apiClients'); + const axios = require('axios'); // Mocked axios + const axiosRetry = require('axios-retry').default; // Mocked axiosRetry + + beforeEach(() => { + // Reset axios.create and axiosRetry mocks + axios.create.mockClear(); + axiosRetry.mockClear(); + // Reconfigure axios.create to return a mock instance with spied interceptors + axios.create.mockImplementation(() => ({ + get: jest.fn(), + interceptors: { + request: { use: jest.fn() }, + response: { use: jest.fn() } + } + })); + }); + + test('should create an instance with provided baseURL and httpsAgent config', () => { + const baseURL = 'https://test.com/api'; + const allowSelfSignedCerts = true; + createApiClientInstance(baseURL, allowSelfSignedCerts); + + expect(axios.create).toHaveBeenCalledTimes(1); + expect(axios.create).toHaveBeenCalledWith(expect.objectContaining({ + baseURL: baseURL, + httpsAgent: expect.objectContaining({ + options: expect.objectContaining({ rejectUnauthorized: false }) + }), + headers: { 'Content-Type': 'application/json' } + })); + }); + + test('should call request.use when authInterceptor is provided', () => { + const mockInterceptor = jest.fn(); + const apiClient = createApiClientInstance('https://test.com', false, mockInterceptor, null); // Pass null for retryConfig + + expect(apiClient.interceptors.request.use).toHaveBeenCalledTimes(1); + expect(apiClient.interceptors.request.use).toHaveBeenCalledWith(mockInterceptor); + }); + + test('should NOT call request.use when authInterceptor is NOT provided', () => { + const apiClient = createApiClientInstance('https://test.com', false, null, null); // Pass null for both + + expect(apiClient.interceptors.request.use).not.toHaveBeenCalled(); + }); + + test('should call axiosRetry when retryConfig is provided', () => { + const mockRetryConfig = { retries: 5, retryDelayLogger: jest.fn(), retryConditionChecker: jest.fn() }; + const apiClient = createApiClientInstance('https://test.com', false, null, mockRetryConfig); + + expect(axiosRetry).toHaveBeenCalledTimes(1); + expect(axiosRetry).toHaveBeenCalledWith(apiClient, { + retries: mockRetryConfig.retries, + retryDelay: mockRetryConfig.retryDelayLogger, // Now correctly accesses the logger + retryCondition: mockRetryConfig.retryConditionChecker, // Now correctly accesses the checker + }); + }); + + test('should NOT call axiosRetry when retryConfig is NOT provided', () => { + createApiClientInstance('https://test.com', false, null, null); // Pass null for both + + expect(axiosRetry).not.toHaveBeenCalled(); + }); + + }); + // --- pveRetryDelayLogger Tests --- describe('pveRetryDelayLogger', () => { const { pveRetryDelayLogger } = require('../apiClients'); diff --git a/server/tests/config.test.js b/server/tests/config.test.js index 6591c4ec9..acb2ab636 100644 --- a/server/tests/config.test.js +++ b/server/tests/config.test.js @@ -1,5 +1,11 @@ const { loadConfiguration, ConfigurationError } = require('../configLoader'); +// Mock dotenv +jest.mock('dotenv', () => ({ + config: jest.fn(), +})); +const dotenv = require('dotenv'); // require after mock + // Helper function to temporarily set environment variables for a test const setEnvVars = (vars) => { const originalEnv = { ...process.env }; // Store original env @@ -361,4 +367,25 @@ describe('Configuration Loading (loadConfiguration)', () => { expect(() => loadConfiguration()).toThrow(/No enabled Proxmox VE or PBS endpoints could be configured/); }); + // New Test Case for dotenv loading + test('should call dotenv.config() when NODE_ENV is not \'test\'', () => { + const originalNodeEnv = process.env.NODE_ENV; + process.env.NODE_ENV = 'development'; // Set to non-test environment + + // Minimal valid PVE config to allow loadConfiguration to proceed far enough + setEnvVars({ + PROXMOX_HOST: 'pve.example.com', + PROXMOX_TOKEN_ID: 'user@pam!pve', + PROXMOX_TOKEN_SECRET: 'secretpve', + }); + + loadConfiguration(); + + expect(dotenv.config).toHaveBeenCalled(); + + // Restore original NODE_ENV and clear mocks for other tests + process.env.NODE_ENV = originalNodeEnv; + dotenv.config.mockClear(); // Clear the mock for other tests + }); + }); \ No newline at end of file diff --git a/server/tests/dataFetcher.test.js b/server/tests/dataFetcher.test.js index b9ad222f0..bf857b3fb 100644 --- a/server/tests/dataFetcher.test.js +++ b/server/tests/dataFetcher.test.js @@ -206,6 +206,43 @@ describe('Data Fetcher', () => { consoleWarnSpy.mockRestore(); }); + test('should handle missing or invalid data.data for a node resource', async () => { + // Arrange: Use default mock client and a specific node name + const nodeName = 'node-missing-data-data'; + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + + // Mock the /nodes call to return the node + mockPveClientInstance.get.mockResolvedValueOnce({ data: { data: [{ node: nodeName, status: 'online' }] } }); // /nodes + + // Mock the /status call to return data with null data.data (covers lines 108-113) + mockPveClientInstance.get.mockResolvedValueOnce({ data: { data: null } }); // status + + // Mock other calls to succeed with empty data to allow the test to proceed + mockPveClientInstance.get.mockResolvedValueOnce({ data: { data: [] } }); // storage + mockPveClientInstance.get.mockResolvedValueOnce({ data: { data: [] } }); // qemu + mockPveClientInstance.get.mockResolvedValueOnce({ data: { data: [] } }); // lxc + + // Act + const result = await fetchDiscoveryData(mockPveApiClient, mockPbsApiClient); + + // Assert + // Verify the warning was logged by fetchNodeResource (covers line 109) + expect(consoleWarnSpy).toHaveBeenCalledWith( + `[DataFetcher - primary-${nodeName}] Node status data missing or invalid format.` + ); + // Verify that the node was still processed but status data is default (null/0) + expect(result.nodes).toHaveLength(1); + expect(result.nodes[0].node).toBe(nodeName); + expect(result.nodes[0].cpu).toBeNull(); // Should be null due to missing data.data + expect(result.nodes[0].uptime).toBe(0); + // Other fetches should have succeeded with empty data + expect(result.nodes[0].storage).toEqual([]); + expect(result.vms).toHaveLength(0); + expect(result.containers).toHaveLength(0); + + consoleWarnSpy.mockRestore(); + }); + test('should handle API error when fetching /nodes for an endpoint', async () => { // Arrange: Need to define specific mock clients for this test const mockPveClientInstance1 = { get: jest.fn() }; @@ -250,7 +287,7 @@ describe('Data Fetcher', () => { // Should still return data from the successful endpoint (as fetchDataForPveEndpoint returns empty on error) expect(result.nodes).toHaveLength(1); expect(result.nodes[0].node).toBe('node-pve2'); - expect(result.nodes[0].endpointId).toBe('pve2'); // Check endpointName (from config.name) + expect(result.nodes[0].endpointId).toBe('secondary'); // Corrected expectation: Should be the endpointId from the mock client setup expect(result.vms).toHaveLength(0); expect(result.containers).toHaveLength(0); expect(result.pbs).toHaveLength(0); @@ -335,7 +372,44 @@ describe('Data Fetcher', () => { expect(result.pbs).toEqual([]); }); - // --- PBS Integration Tests --- + test('should handle missing or invalid data.data for a node resource', async () => { + // Arrange: Use default mock client and a specific node name + const nodeName = 'node-missing-data-data'; + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + + // Mock the /nodes call to return the node + mockPveClientInstance.get.mockResolvedValueOnce({ data: { data: [{ node: nodeName, status: 'online' }] } }); // /nodes + + // Mock the /status call to return data with null data.data (covers lines 108-113) + mockPveClientInstance.get.mockResolvedValueOnce({ data: { data: null } }); // status + + // Mock other calls to succeed with empty data to allow the test to proceed + mockPveClientInstance.get.mockResolvedValueOnce({ data: { data: [] } }); // storage + mockPveClientInstance.get.mockResolvedValueOnce({ data: { data: [] } }); // qemu + mockPveClientInstance.get.mockResolvedValueOnce({ data: { data: [] } }); // lxc + + // Act + const result = await fetchDiscoveryData(mockPveApiClient, mockPbsApiClient); + + // Assert + // Verify the warning was logged by fetchNodeResource (covers line 109) + expect(consoleWarnSpy).toHaveBeenCalledWith( + `[DataFetcher - primary-${nodeName}] Node status data missing or invalid format.` + ); + // Verify that the node was still processed but status data is default (null/0) + expect(result.nodes).toHaveLength(1); + expect(result.nodes[0].node).toBe(nodeName); + expect(result.nodes[0].cpu).toBeNull(); // Should be null due to missing data.data + expect(result.nodes[0].uptime).toBe(0); + // Other fetches should have succeeded with empty data + expect(result.nodes[0].storage).toEqual([]); + expect(result.vms).toHaveLength(0); + expect(result.containers).toHaveLength(0); + + consoleWarnSpy.mockRestore(); + }); + + // --- PBS Integration Tests --- test('should fetch PVE and PBS data correctly', async () => { // Arrange PVE (similar to happy path test, simplified) const nodeName = 'pve-node'; @@ -1696,6 +1770,55 @@ describe('Data Fetcher', () => { consoleErrorSpy.mockRestore(); }); + test('should handle API error when fetching PBS tasks', async () => { + const pbsId = 'pbs-task-fetch-error'; + const pbsNodeName = 'pbs-node-task-error'; + const datastoreName = 'store-task-error'; + const mockPbsClient = { get: jest.fn() }; + const mockClients = { [pbsId]: { client: mockPbsClient, config: { name: 'PBS Task Fetch Error' } } }; + const taskError = new Error('Simulated task fetch error'); + + mockPbsClient.get + .mockResolvedValueOnce({ data: { data: [{ node: pbsNodeName }] } }) // /nodes (succeeds) + .mockResolvedValueOnce({ data: { data: [{ store: datastoreName, total: 1, used: 0 }] } }) // /status/datastore-usage (succeeds) + .mockResolvedValueOnce({ data: { data: [] } }) // Snapshots (succeeds empty) + .mockRejectedValueOnce(taskError); // /nodes/{node}/tasks (FAILS) + + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + + const result = await fetchPbsData(mockClients); + + expect(result).toHaveLength(1); + expect(result[0]).toMatchObject({ + pbsEndpointId: pbsId, + pbsInstanceName: 'PBS Task Fetch Error', + status: 'ok', // Status remains 'ok' as node and datastores (even if empty) fetched + nodeName: pbsNodeName, + datastores: [{ name: datastoreName, total: 1, used: 0, available: undefined, gcStatus: 'unknown' , snapshots: []}], // Datastore fetch succeeded + }); + // Check that task-related properties are NOT added or are default due to fetch error + expect(result[0].backupTasks).toBeUndefined(); + expect(result[0].verifyTasks).toBeUndefined(); + expect(result[0].gcTasks).toBeUndefined(); + + // Verify API calls + expect(mockPbsClient.get).toHaveBeenCalledTimes(4); // nodes, usage, snapshots, tasks + expect(mockPbsClient.get).toHaveBeenCalledWith(`/nodes/${pbsNodeName}/tasks`, expect.any(Object)); // Check tasks call was attempted + + // Verify error logging from fetchAllPbsTasksForProcessing's catch block (covers lines 264-265) + expect(consoleErrorSpy).toHaveBeenCalledWith( + expect.stringContaining(`ERROR: [DataFetcher] Failed to fetch PBS task list for node ${pbsNodeName} (PBS Task Fetch Error): ${taskError.message}`) + ); + // Verify the warning logged in fetchPbsData when tasks cannot be processed (covers line 341) + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining('WARN: [DataFetcher - PBS Task Fetch Error] No tasks to process or task fetching failed. Error flag: true, Tasks array: null') + ); + + consoleErrorSpy.mockRestore(); + consoleWarnSpy.mockRestore(); + }); + }); // End describe fetchPbsData }); // End describe Data Fetcher diff --git a/server/tests/pbsUtils.test.js b/server/tests/pbsUtils.test.js index 2e6f46eb1..7232e183a 100644 --- a/server/tests/pbsUtils.test.js +++ b/server/tests/pbsUtils.test.js @@ -29,6 +29,7 @@ describe('PBS Utils - processPbsTasks', () => { { upid: 'B1', worker_type: 'backup', status: 'OK', starttime: now - 3600, endtime: now - 3500 }, { upid: 'B2', type: 'backup', status: 'OK', starttime: now - 7200, endtime: now - 7100 }, { upid: 'B3', worker_type: 'backup', status: 'FAILED', starttime: now - 100, endtime: now - 50 }, + { upid: 'B4', worker_type: 'backup', status: 'ERROR', starttime: now - 40, endtime: now - 20 }, // Verifications { upid: 'V1', worker_type: 'verify', status: 'OK', starttime: now - 500, endtime: now - 400 }, { upid: 'V2', type: 'verificationjob', status: 'WARNING', starttime: now - 600, endtime: now - 550 }, // Treated as failed @@ -47,11 +48,11 @@ describe('PBS Utils - processPbsTasks', () => { // Backup Summary expect(result.backupTasks.summary.ok).toBe(2); - expect(result.backupTasks.summary.failed).toBe(1); - expect(result.backupTasks.summary.total).toBe(3); + expect(result.backupTasks.summary.failed).toBe(2); + expect(result.backupTasks.summary.total).toBe(4); expect(result.backupTasks.summary.lastOk).toBe(now - 3500); - expect(result.backupTasks.summary.lastFailed).toBe(now - 50); - expect(result.backupTasks.recentTasks).toHaveLength(4); // B3, B1, R1, B2 (all backup tasks included) + expect(result.backupTasks.summary.lastFailed).toBe(now - 20); + expect(result.backupTasks.recentTasks).toHaveLength(5); // Verification Summary expect(result.verificationTasks.summary.ok).toBe(1); @@ -187,4 +188,14 @@ describe('PBS Utils - processPbsTasks', () => { expect(result.pruneTasks.recentTasks.map(t => t.upid)).toEqual(['P1', 'G1']); // Sorted by start time }); + test('should return default structure for non-array input', () => { + const result = processPbsTasks({}); // Pass an object instead of an array + expect(result).toEqual({ + backupTasks: { recentTasks: [], summary: { ok: 0, failed: 0, total: 0, lastOk: null, lastFailed: null } }, + verificationTasks: { recentTasks: [], summary: { ok: 0, failed: 0, total: 0, lastOk: null, lastFailed: null } }, + syncTasks: { recentTasks: [], summary: { ok: 0, failed: 0, total: 0, lastOk: null, lastFailed: null } }, + pruneTasks: { recentTasks: [], summary: { ok: 0, failed: 0, total: 0, lastOk: null, lastFailed: null } } + }); + }); + });