Fix: Correct syntax error and update tests

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');

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
+  });
+
 }); 

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

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 } }
+        });
+    });
+
 });