test: fix connect plugin tests (#1441)

## Summary by CodeRabbit

* **Bug Fixes**
* Improved error handling to prevent runtime exceptions when certain
configuration fields are undefined.
  * Updated test expectations to reflect new error handling behaviors.

* **Refactor**
* Streamlined and modularized the legacy configuration migration process
for better maintainability.
* Removed obsolete configuration fields and related validation from the
configuration model.

* **Tests**
* Added and enhanced test suites to verify configuration migration,
parsing, and URL resolution logic.
* Improved test coverage and robustness with expanded mock data and
flexible assertions.

* **Chores**
* Updated test scripts and dependencies for improved reliability and
compatibility.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Pujit Mehrotra
2025-06-26 15:04:26 -04:00
committed by GitHub
parent c4c99843c7
commit af6e56de60
7 changed files with 242 additions and 47 deletions

View File

@@ -72,6 +72,9 @@ jobs:
- name: PNPM Install
run: pnpm install --frozen-lockfile
- name: Lint
run: pnpm run lint
- name: Setup libvirt
run: |
# Create required groups (if they don't already exist)
@@ -111,11 +114,41 @@ jobs:
# Verify libvirt is running using sudo to bypass group membership delays
sudo virsh list --all || true
- name: Lint
run: pnpm run lint
- uses: oven-sh/setup-bun@v2
with:
bun-version: latest
- name: Test
run: pnpm run coverage
- name: Run Tests Concurrently
run: |
set -e
# Run all tests in parallel with labeled output
echo "🚀 Starting API coverage tests..."
pnpm run coverage > api-test.log 2>&1 &
API_PID=$!
echo "🚀 Starting Connect plugin tests..."
(cd ../packages/unraid-api-plugin-connect && pnpm test) > connect-test.log 2>&1 &
CONNECT_PID=$!
echo "🚀 Starting Shared package tests..."
(cd ../packages/unraid-shared && pnpm test) > shared-test.log 2>&1 &
SHARED_PID=$!
# Wait for all processes and capture exit codes
wait $API_PID && echo "✅ API tests completed" || { echo "❌ API tests failed"; API_EXIT=1; }
wait $CONNECT_PID && echo "✅ Connect tests completed" || { echo "❌ Connect tests failed"; CONNECT_EXIT=1; }
wait $SHARED_PID && echo "✅ Shared tests completed" || { echo "❌ Shared tests failed"; SHARED_EXIT=1; }
# Display all outputs
echo "📋 API Test Results:" && cat api-test.log
echo "📋 Connect Plugin Test Results:" && cat connect-test.log
echo "📋 Shared Package Test Results:" && cat shared-test.log
# Exit with error if any test failed
if [[ ${API_EXIT:-0} -eq 1 || ${CONNECT_EXIT:-0} -eq 1 || ${SHARED_EXIT:-0} -eq 1 ]]; then
exit 1
fi
build-api:
name: Build API

View File

@@ -0,0 +1,137 @@
import { ConfigService } from '@nestjs/config';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { ApiConfigPersistence } from '@app/unraid-api/config/api-config.module.js';
import { ConfigPersistenceHelper } from '@app/unraid-api/config/persistence.helper.js';
describe('ApiConfigPersistence', () => {
let service: ApiConfigPersistence;
let configService: ConfigService;
let persistenceHelper: ConfigPersistenceHelper;
beforeEach(() => {
configService = {
get: vi.fn(),
set: vi.fn(),
} as any;
persistenceHelper = {} as ConfigPersistenceHelper;
service = new ApiConfigPersistence(configService, persistenceHelper);
});
describe('convertLegacyConfig', () => {
it('should migrate sandbox from string "yes" to boolean true', () => {
const legacyConfig = {
local: { sandbox: 'yes' },
api: { extraOrigins: '' },
remote: { ssoSubIds: '' },
};
const result = service.convertLegacyConfig(legacyConfig);
expect(result.sandbox).toBe(true);
});
it('should migrate sandbox from string "no" to boolean false', () => {
const legacyConfig = {
local: { sandbox: 'no' },
api: { extraOrigins: '' },
remote: { ssoSubIds: '' },
};
const result = service.convertLegacyConfig(legacyConfig);
expect(result.sandbox).toBe(false);
});
it('should migrate extraOrigins from comma-separated string to array', () => {
const legacyConfig = {
local: { sandbox: 'no' },
api: { extraOrigins: 'https://example.com,https://test.com' },
remote: { ssoSubIds: '' },
};
const result = service.convertLegacyConfig(legacyConfig);
expect(result.extraOrigins).toEqual(['https://example.com', 'https://test.com']);
});
it('should filter out non-HTTP origins from extraOrigins', () => {
const legacyConfig = {
local: { sandbox: 'no' },
api: {
extraOrigins: 'https://example.com,invalid-origin,http://test.com,ftp://bad.com',
},
remote: { ssoSubIds: '' },
};
const result = service.convertLegacyConfig(legacyConfig);
expect(result.extraOrigins).toEqual(['https://example.com', 'http://test.com']);
});
it('should handle empty extraOrigins string', () => {
const legacyConfig = {
local: { sandbox: 'no' },
api: { extraOrigins: '' },
remote: { ssoSubIds: '' },
};
const result = service.convertLegacyConfig(legacyConfig);
expect(result.extraOrigins).toEqual([]);
});
it('should migrate ssoSubIds from comma-separated string to array', () => {
const legacyConfig = {
local: { sandbox: 'no' },
api: { extraOrigins: '' },
remote: { ssoSubIds: 'user1,user2,user3' },
};
const result = service.convertLegacyConfig(legacyConfig);
expect(result.ssoSubIds).toEqual(['user1', 'user2', 'user3']);
});
it('should handle empty ssoSubIds string', () => {
const legacyConfig = {
local: { sandbox: 'no' },
api: { extraOrigins: '' },
remote: { ssoSubIds: '' },
};
const result = service.convertLegacyConfig(legacyConfig);
expect(result.ssoSubIds).toEqual([]);
});
it('should handle undefined config sections', () => {
const legacyConfig = {};
const result = service.convertLegacyConfig(legacyConfig);
expect(result.sandbox).toBe(false);
expect(result.extraOrigins).toEqual([]);
expect(result.ssoSubIds).toEqual([]);
});
it('should handle complete migration with all fields', () => {
const legacyConfig = {
local: { sandbox: 'yes' },
api: { extraOrigins: 'https://app1.example.com,https://app2.example.com' },
remote: { ssoSubIds: 'sub1,sub2,sub3' },
};
const result = service.convertLegacyConfig(legacyConfig);
expect(result.sandbox).toBe(true);
expect(result.extraOrigins).toEqual([
'https://app1.example.com',
'https://app2.example.com',
]);
expect(result.ssoSubIds).toEqual(['sub1', 'sub2', 'sub3']);
});
});
});

View File

@@ -8,7 +8,7 @@
"readme.md"
],
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1",
"test": "vitest",
"clean": "rimraf dist",
"build": "tsc",
"prepare": "npm run build",
@@ -60,7 +60,7 @@
"rxjs": "^7.8.2",
"type-fest": "^4.37.0",
"typescript": "^5.8.2",
"vitest": "^3.1.4",
"vitest": "^3.2.4",
"ws": "^8.18.0",
"zen-observable-ts": "^1.1.0"
},

View File

@@ -328,7 +328,7 @@ export class UrlResolverService {
});
// Now Process the FQDN Urls
nginx.fqdnUrls.forEach((fqdnUrl: FqdnEntry) => {
nginx.fqdnUrls?.forEach((fqdnUrl: FqdnEntry) => {
doSafely(() => {
const urlType = this.getUrlTypeFromFqdn(fqdnUrl.interface);
const fqdnUrlToUse = this.getUrlForField({

View File

@@ -32,14 +32,12 @@ describe('CloudService.hardCheckCloud (integration)', () => {
it('fails to authenticate with mothership with no credentials', async () => {
try {
await expect(
service['hardCheckCloud'](API_VERSION, BAD)
).rejects.toThrowErrorMatchingInlineSnapshot(
`[Error: Failed to connect to https://mothership.unraid.net/ws with a "426" HTTP error.]`
);
await expect(
service['hardCheckCloud'](API_VERSION, BAD_API_KEY)
).rejects.toThrowErrorMatchingInlineSnapshot(`[Error: Invalid credentials]`);
await expect(service['hardCheckCloud'](API_VERSION, BAD)).resolves.toMatchObject({
status: 'error',
});
await expect(service['hardCheckCloud'](API_VERSION, BAD_API_KEY)).resolves.toMatchObject({
status: 'error',
});
} catch (error) {
if (error instanceof Error && error.message.includes('Timeout')) {
// Test succeeds on timeout

View File

@@ -19,6 +19,7 @@ describe('UrlResolverService', () => {
beforeEach(() => {
mockConfigService = {
get: vi.fn(),
getOrThrow: vi.fn(),
} as unknown as ConfigService<ConfigType, true>;
service = new UrlResolverService(mockConfigService);
@@ -27,6 +28,7 @@ describe('UrlResolverService', () => {
describe('getServerIps', () => {
it('should return empty arrays when store is not loaded', () => {
(mockConfigService.get as Mock).mockReturnValue(null);
(mockConfigService.getOrThrow as Mock).mockReturnValue(null);
const result = service.getServerIps();
@@ -59,11 +61,16 @@ describe('UrlResolverService', () => {
const mockStore = {
emhttp: {
nginx: {
defaultUrl: 'https://default.unraid.net',
lanIp: '192.168.1.1',
lanIp6: '2001:db8::1',
lanName: 'unraid.local',
lanMdns: 'unraid.local',
sslEnabled: true,
sslMode: 'yes',
httpPort,
httpsPort,
fqdnUrls: [],
},
},
};
@@ -88,10 +95,15 @@ describe('UrlResolverService', () => {
emhttp: {
nginx: {
defaultUrl: 'https://BROKEN_URL',
lanIp: '192.168.1.1',
lanIp6: '2001:db8::1',
lanName: 'unraid.local',
lanMdns: 'unraid.local',
sslEnabled: true,
sslMode: 'yes',
httpPort: 80,
httpsPort: 443,
fqdnUrls: [],
},
},
};
@@ -99,8 +111,8 @@ describe('UrlResolverService', () => {
(mockConfigService.get as Mock).mockReturnValue(mockStore);
const result = service.getServerIps();
expect(result.errors).toHaveLength(1);
expect(result.errors[0].message).toContain('Failed to parse URL');
expect(result.errors.length).toBeGreaterThan(0);
expect(result.errors.some(error => error.message.includes('Failed to parse URL'))).toBe(true);
});
it('should handle SSL mode variations', () => {
@@ -128,11 +140,16 @@ describe('UrlResolverService', () => {
const mockStore = {
emhttp: {
nginx: {
defaultUrl: 'https://default.unraid.net',
lanIp: '192.168.1.1',
lanIp6: '2001:db8::1',
lanName: 'unraid.local',
lanMdns: 'unraid.local',
sslEnabled: testCase.sslEnabled,
sslMode: testCase.sslMode,
httpPort: 80,
httpsPort: 443,
fqdnUrls: [],
},
},
};
@@ -142,8 +159,7 @@ describe('UrlResolverService', () => {
const result = service.getServerIps();
if (testCase.shouldError) {
expect(result.errors).toHaveLength(1);
expect(result.errors[0].message).toContain('SSL mode auto');
expect(result.errors.some(error => error.message.includes('SSL mode auto'))).toBe(true);
} else {
const lanUrl = result.urls.find(
(url) => url.type === URL_TYPE.LAN && url.name === 'LAN IPv4'
@@ -160,7 +176,7 @@ describe('UrlResolverService', () => {
nginx: {
defaultUrl: 'https://default.unraid.net',
lanIp: '192.168.1.1',
lanIp6: '2001:db8::1',
lanIp6: 'ipv6.unraid.local',
lanName: 'unraid.local',
lanMdns: 'unraid.local',
sslEnabled: true,
@@ -183,18 +199,14 @@ describe('UrlResolverService', () => {
],
},
},
config: {
remote: {
wanport: 443,
},
},
};
(mockConfigService.get as Mock).mockReturnValue(mockStore);
(mockConfigService.getOrThrow as Mock).mockReturnValue(443);
const result = service.getServerIps();
expect(result.urls).toHaveLength(6); // Default + LAN IPv4 + LAN IPv6 + LAN Name + LAN MDNS + 2 FQDN
expect(result.urls).toHaveLength(7); // Default + LAN IPv4 + LAN IPv6 + LAN Name + LAN MDNS + 2 FQDN
expect(result.errors).toHaveLength(0);
// Verify default URL
@@ -214,7 +226,7 @@ describe('UrlResolverService', () => {
(url) => url.type === URL_TYPE.LAN && url.name === 'LAN IPv6'
);
expect(lanIp6Url).toBeDefined();
expect(lanIp6Url?.ipv4?.toString()).toBe('https://2001:db8::1/');
expect(lanIp6Url?.ipv6?.toString()).toBe('https://ipv6.unraid.local/');
// Verify LAN Name URL
const lanNameUrl = result.urls.find(
@@ -250,31 +262,51 @@ describe('UrlResolverService', () => {
const mockStore = {
emhttp: {
nginx: {
wanIp: '1.2.3.4',
defaultUrl: 'https://default.unraid.net',
lanIp: '192.168.1.1',
lanIp6: '2001:db8::1',
lanName: 'unraid.local',
lanMdns: 'unraid.local',
sslEnabled: true,
sslMode: 'yes',
httpPort: 80,
httpsPort: 443,
fqdnUrls: [
{
interface: 'WAN',
id: null,
fqdn: 'wan.unraid.net',
isIpv6: false,
},
],
},
},
};
(mockConfigService.get as Mock).mockReturnValue(mockStore);
(mockConfigService.getOrThrow as Mock).mockReturnValue(443);
const result = service.getRemoteAccessUrl();
expect(result).toBeDefined();
expect(result?.type).toBe(URL_TYPE.WAN);
expect(result?.ipv4?.toString()).toBe('https://1.2.3.4/');
expect(result?.ipv4?.toString()).toBe('https://wan.unraid.net/');
});
it('should return null when no WAN URL is available', () => {
const mockStore = {
emhttp: {
nginx: {
defaultUrl: 'https://default.unraid.net',
lanIp: '192.168.1.1',
lanIp6: '2001:db8::1',
lanName: 'unraid.local',
lanMdns: 'unraid.local',
sslEnabled: true,
sslMode: 'yes',
httpPort: 80,
httpsPort: 443,
fqdnUrls: [],
},
},
};

31
pnpm-lock.yaml generated
View File

@@ -612,7 +612,7 @@ importers:
specifier: ^5.8.2
version: 5.8.3
vitest:
specifier: ^3.1.4
specifier: ^3.2.4
version: 3.2.4(@types/node@22.15.32)(@vitest/ui@3.2.4)(happy-dom@18.0.0)(jiti@2.4.2)(jsdom@26.1.0)(stylus@0.57.0)(terser@5.43.1)(tsx@4.20.3)(yaml@2.8.0)
ws:
specifier: ^8.18.0
@@ -949,7 +949,7 @@ importers:
version: 8.6.14(prettier@3.5.3)
tailwind-rem-to-rem:
specifier: github:unraid/tailwind-rem-to-rem
version: '@unraid/tailwind-rem-to-rem@https://codeload.github.com/unraid/tailwind-rem-to-rem/tar.gz/4b907d0cdb3abda88de9813e33c13c3e7b1300c4(tailwindcss@3.4.17(ts-node@10.9.2(@swc/core@1.12.4)(@types/node@22.15.32)(typescript@5.8.3)))'
version: '@unraid/tailwind-rem-to-rem@https://codeload.github.com/unraid/tailwind-rem-to-rem/tar.gz/54ab7d5c7b054c4c727cb64ee823b09d2c48c1b3(tailwindcss@3.4.17(ts-node@10.9.2(@swc/core@1.12.4)(@types/node@22.15.32)(typescript@5.8.3)))'
tailwindcss:
specifier: ^3.0.0
version: 3.4.17(ts-node@10.9.2(@swc/core@1.12.4)(@types/node@22.15.32)(typescript@5.8.3))
@@ -4844,11 +4844,11 @@ packages:
peerDependencies:
tailwindcss: ^3.4.17
'@unraid/tailwind-rem-to-rem@https://codeload.github.com/unraid/tailwind-rem-to-rem/tar.gz/4b907d0cdb3abda88de9813e33c13c3e7b1300c4':
resolution: {tarball: https://codeload.github.com/unraid/tailwind-rem-to-rem/tar.gz/4b907d0cdb3abda88de9813e33c13c3e7b1300c4}
version: 1.1.0
'@unraid/tailwind-rem-to-rem@https://codeload.github.com/unraid/tailwind-rem-to-rem/tar.gz/54ab7d5c7b054c4c727cb64ee823b09d2c48c1b3':
resolution: {tarball: https://codeload.github.com/unraid/tailwind-rem-to-rem/tar.gz/54ab7d5c7b054c4c727cb64ee823b09d2c48c1b3}
version: 2.0.0
peerDependencies:
tailwindcss: ^3.4.17
tailwindcss: ^4.0.0
'@unrs/resolver-binding-android-arm-eabi@1.9.1':
resolution: {integrity: sha512-dd7yIp1hfJFX9ZlVLQRrh/Re9WMUHHmF9hrKD1yIvxcyNr2BhQ3xc1upAVhy8NijadnCswAxWQu8MkkSMC1qXQ==}
@@ -9535,9 +9535,6 @@ packages:
resolution: {integrity: sha512-lyuxPGr/Wfhrlem2CL/UcnUc1zcqKAImBDzukY7Y5F/yQiNdko6+fRLevlw1HgMySw7f611UIY408EtxRSoK3Q==}
hasBin: true
loupe@3.1.3:
resolution: {integrity: sha512-kkIp7XSkP78ZxJEsSxW3712C6teJVoeHHwgo9zJ380de7IYyJ2ISlxojcH2pC5OFLewESmnRi/+XCDIEEVyoug==}
loupe@3.1.4:
resolution: {integrity: sha512-wJzkKwJrheKtknCOKNEtDK4iqg/MxmZheEMtSTYvnzRdEYaZzmgH976nenp8WdJRdx5Vc1X/9MO0Oszl6ezeXg==}
@@ -14685,7 +14682,7 @@ snapshots:
'@eslint/config-array@0.20.1':
dependencies:
'@eslint/object-schema': 2.1.6
debug: 4.4.0(supports-color@5.5.0)
debug: 4.4.1
minimatch: 3.1.2
transitivePeerDependencies:
- supports-color
@@ -14730,7 +14727,7 @@ snapshots:
'@eslint/eslintrc@3.3.1':
dependencies:
ajv: 6.12.6
debug: 4.4.0(supports-color@5.5.0)
debug: 4.4.1
espree: 10.4.0
globals: 14.0.0
ignore: 5.3.2
@@ -17795,7 +17792,7 @@ snapshots:
dependencies:
tailwindcss: 3.4.17(ts-node@10.9.2(@swc/core@1.12.4)(@types/node@22.15.32)(typescript@5.8.3))
'@unraid/tailwind-rem-to-rem@https://codeload.github.com/unraid/tailwind-rem-to-rem/tar.gz/4b907d0cdb3abda88de9813e33c13c3e7b1300c4(tailwindcss@3.4.17(ts-node@10.9.2(@swc/core@1.12.4)(@types/node@22.15.32)(typescript@5.8.3)))':
'@unraid/tailwind-rem-to-rem@https://codeload.github.com/unraid/tailwind-rem-to-rem/tar.gz/54ab7d5c7b054c4c727cb64ee823b09d2c48c1b3(tailwindcss@3.4.17(ts-node@10.9.2(@swc/core@1.12.4)(@types/node@22.15.32)(typescript@5.8.3)))':
dependencies:
tailwindcss: 3.4.17(ts-node@10.9.2(@swc/core@1.12.4)(@types/node@22.15.32)(typescript@5.8.3))
@@ -19178,7 +19175,7 @@ snapshots:
assertion-error: 2.0.1
check-error: 2.1.1
deep-eql: 5.0.2
loupe: 3.1.3
loupe: 3.1.4
pathval: 2.0.0
chalk@2.4.2:
@@ -23167,8 +23164,6 @@ snapshots:
dependencies:
js-tokens: 4.0.0
loupe@3.1.3: {}
loupe@3.1.4: {}
lower-case-first@2.0.2:
@@ -24438,7 +24433,7 @@ snapshots:
pm2-axon-rpc@0.7.1:
dependencies:
debug: 4.4.0(supports-color@5.5.0)
debug: 4.4.1
transitivePeerDependencies:
- supports-color
@@ -24446,7 +24441,7 @@ snapshots:
dependencies:
amp: 0.3.1
amp-message: 0.1.2
debug: 4.4.0(supports-color@5.5.0)
debug: 4.4.1
escape-string-regexp: 4.0.0
transitivePeerDependencies:
- supports-color
@@ -24463,7 +24458,7 @@ snapshots:
pm2-sysmonit@1.2.8:
dependencies:
async: 3.2.6
debug: 4.4.0(supports-color@5.5.0)
debug: 4.4.1
pidusage: 2.0.21
systeminformation: 5.27.6
tx2: 1.0.5