fix(detector): Improve test coverage and adjust detection result handling

changelog.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## 2025-05-26 - 2.1.1 - fix(detector)
+Improve test coverage and adjust detection result handling
+
+- Updated readme hints and plan to document the enhanced comprehensive test suite
+- Expanded tests to cover backward compatibility, performance benchmarks, and multiple service types
+- Modified detection logic to set serviceType to UNKNOWN when a port is inactive with detectServiceType option
+
 ## 2025-05-26 - 2.1.0 - feat(detector)
 Enhance port detection and service fingerprinting with improved HTTP/HTTPS and SSH checks, update test scripts for verbose output, and revise documentation with new hints and a detailed improvement plan.
 

readme.hints.md

@@ -37,11 +37,17 @@
 - **v2.0.2**: Added service type detection, protocol fingerprinting, and enhanced API
 
 ## Testing
-- Tests check for closed local ports and open remote ports
-- Tests verify service type detection for HTTP/HTTPS
-- Tests check SSH service detection
+- Comprehensive test suite with 19 tests covering:
+  - Basic port detection (local and remote)
+  - Backward compatibility with `isActiveSimple()`
+  - Service type detection for HTTP, HTTPS, SSH
+  - Error handling and edge cases
+  - Performance benchmarks
+  - Common development ports
+  - Database service detection (MySQL, Redis)
 - Uses `@git.zone/tstest` with tap-style testing
-- Example tests: localhost:3008 (expects closed), lossless.com (expects open)
+- Tests ensure serviceType is always returned when detectServiceType option is true
+- Handles non-standard URL schemes with default ports
 
 ## Notes
 - Project uses pnpm for package management

readme.plan.md

@@ -66,4 +66,4 @@ Command to reread CLAUDE.md: `cat ~/.claude/CLAUDE.md`
 - ✅ IDetectorOptions for configuration
 - ✅ Backward compatibility with `isActiveSimple()` method
 - ✅ Banner grabbing for unknown services
-- ✅ Tests for new functionality
+- ✅ Tests for new functionality - IMPROVED with comprehensive test suite (19 tests)

test/test.ts

@@ -3,14 +3,16 @@ import * as detector from '../ts/index.js';
 
 let testDetector: detector.Detector;
 
-tap.test('first test', async () => {
+tap.test('should create a detector instance', async () => {
   testDetector = new detector.Detector();
   expect(testDetector).toBeInstanceOf(detector.Detector);
 });
 
-tap.test('should detect an closed port on a local domain', async () => {
+// Basic port detection tests
+tap.test('should detect a closed port on localhost', async () => {
   const result = await testDetector.isActive('http://localhost:3008');
   expect(result.isActive).toBeFalse();
+  expect(result.serviceType).toBeUndefined();
 });
 
 tap.test('should detect an open port on a remote domain', async () => {
@@ -18,28 +20,130 @@ tap.test('should detect an open port on a remote domain', async () => {
   expect(result.isActive).toBeTrue();
 });
 
-tap.test('should detect service type for HTTP', async () => {
+// Backward compatibility tests
+tap.test('should support backward compatibility with isActiveSimple', async () => {
+  const result = await testDetector.isActiveSimple('https://example.com');
+  expect(result).toBeTypeofBoolean();
+  expect(result).toBeTrue();
+});
+
+tap.test('should return false for closed port with isActiveSimple', async () => {
+  const result = await testDetector.isActiveSimple('http://localhost:3008');
+  expect(result).toBeFalse();
+});
+
+// Service detection tests
+tap.test('should detect HTTP service on port 80', async () => {
   const result = await testDetector.isActive('http://example.com', { detectServiceType: true });
   expect(result.isActive).toBeTrue();
   expect(result.serviceType).toEqual(detector.ServiceType.HTTP);
 });
 
-tap.test('should detect service type for HTTPS', async () => {
+tap.test('should detect HTTPS service on port 443', async () => {
   const result = await testDetector.isActive('https://example.com', { detectServiceType: true });
   expect(result.isActive).toBeTrue();
   expect(result.serviceType).toEqual(detector.ServiceType.HTTPS);
 });
 
-tap.test('should detect SSH service', async () => {
+tap.test('should detect SSH service on GitHub', async () => {
   const sshType = await testDetector.detectType('ssh://github.com:22');
   expect(sshType).toEqual(detector.ServiceType.SSH);
 });
 
-tap.test('should return unknown for non-standard services', async () => {
+tap.test('should detect HTTPS on non-standard port', async () => {
+  const result = await testDetector.isActive('https://lossless.com:443', { detectServiceType: true });
+  if (result.isActive) {
+    expect(result.serviceType).toEqual(detector.ServiceType.HTTPS);
+  }
+});
+
+// Direct detectType tests
+tap.test('should detect common services by port number', async () => {
+  // Test FTP port
+  const ftpType = await testDetector.detectType('ftp://localhost:21');
+  // Since localhost:21 is likely not running, it will try detection
+  expect(ftpType).toBeTypeofString();
+});
+
+tap.test('should return UNKNOWN for non-standard ports', async () => {
   const result = await testDetector.isActive('http://localhost:9999', { detectServiceType: true });
   if (result.isActive) {
     expect(result.serviceType).toEqual(detector.ServiceType.UNKNOWN);
   }
 });
 
+// Edge cases
+tap.test('should handle invalid URLs gracefully', async () => {
+  try {
+    await testDetector.isActive('not-a-valid-url');
+  } catch (error) {
+    expect(error).toBeInstanceOf(Error);
+  }
+});
+
+tap.test('should handle localhost with detectServiceType', async () => {
+  const result = await testDetector.isActive('http://localhost:8080', { detectServiceType: true });
+  expect(result).toHaveProperty('isActive');
+  expect(result).toHaveProperty('serviceType');
+});
+
+// Multiple service checks
+tap.test('should correctly identify multiple HTTPS services', async () => {
+  const services = [
+    { url: 'https://google.com', expected: detector.ServiceType.HTTPS },
+    { url: 'https://github.com', expected: detector.ServiceType.HTTPS },
+  ];
+
+  for (const service of services) {
+    const result = await testDetector.isActive(service.url, { detectServiceType: true });
+    if (result.isActive) {
+      expect(result.serviceType).toEqual(service.expected);
+    }
+  }
+});
+
+// Performance test
+tap.test('should complete detection within reasonable time', async () => {
+  const startTime = Date.now();
+  await testDetector.isActive('https://example.com', { detectServiceType: true });
+  const duration = Date.now() - startTime;
+  // Should complete within 10 seconds
+  expect(duration).toBeLessThan(10000);
+});
+
+// Test without service detection
+tap.test('should work without service detection option', async () => {
+  const result = await testDetector.isActive('https://example.com');
+  expect(result.isActive).toBeTrue();
+  expect(result.serviceType).toBeUndefined();
+});
+
+// Test specific ports
+tap.test('should handle MySQL default port', async () => {
+  const mysqlType = await testDetector.detectType('mysql://localhost:3306');
+  // Will return MYSQL based on port, but actual detection depends on service running
+  expect([detector.ServiceType.MYSQL, detector.ServiceType.UNKNOWN]).toContain(mysqlType);
+});
+
+tap.test('should handle Redis default port', async () => {
+  const redisType = await testDetector.detectType('redis://localhost:6379');
+  expect([detector.ServiceType.REDIS, detector.ServiceType.UNKNOWN]).toContain(redisType);
+});
+
+// Local port tests
+tap.test('should detect commonly used local development ports', async () => {
+  const localPorts = [
+    { url: 'http://localhost:3000', name: 'Node.js dev server' },
+    { url: 'http://localhost:4200', name: 'Angular dev server' },
+    { url: 'http://localhost:8080', name: 'Common web server' },
+  ];
+
+  for (const port of localPorts) {
+    const result = await testDetector.isActive(port.url);
+    // Just check that it returns a valid result structure
+    expect(result).toHaveProperty('isActive');
+    expect(result.isActive).toBeTypeofBoolean();
+  }
+});
+
 export default tap.start();

ts/00_commitinfo_data.ts

@@ -3,6 +3,6 @@
  */
 export const commitinfo = {
   name: '@uptime.link/detector',
-  version: '2.1.0',
+  version: '2.1.1',
   description: 'a detector for answering network questions locally. It does not rely on any online services.'
 }

ts/detector.classes.detector.ts

@@ -29,9 +29,13 @@ export class Detector {
         isActive: portAvailable
       };
       
-      if (portAvailable && options?.detectServiceType) {
-        const serviceType = await this.detectType(urlArg);
-        result.serviceType = serviceType;
+      if (options?.detectServiceType) {
+        if (portAvailable) {
+          const serviceType = await this.detectType(urlArg);
+          result.serviceType = serviceType;
+        } else {
+          result.serviceType = ServiceType.UNKNOWN;
+        }
       }
       
       return result;
@@ -46,9 +50,13 @@ export class Detector {
         isActive: portAvailable
       };
       
-      if (portAvailable && options?.detectServiceType) {
-        const serviceType = await this.detectType(urlArg);
-        result.serviceType = serviceType;
+      if (options?.detectServiceType) {
+        if (portAvailable) {
+          const serviceType = await this.detectType(urlArg);
+          result.serviceType = serviceType;
+        } else {
+          result.serviceType = ServiceType.UNKNOWN;
+        }
       }
       
       return result;
@@ -57,7 +65,27 @@ export class Detector {
 
   public async detectType(urlArg: string): Promise<ServiceType> {
     const parsedUrl = plugins.smarturl.Smarturl.createFromUrl(urlArg);
-    const port = parseInt(parsedUrl.port, 10);
+    
+    // Handle different URL schemes and default ports
+    let port = parseInt(parsedUrl.port, 10);
+    if (isNaN(port)) {
+      // Set default ports based on scheme
+      const defaultPorts: { [key: string]: number } = {
+        'http': 80,
+        'https': 443,
+        'ssh': 22,
+        'ftp': 21,
+        'smtp': 25,
+        'mysql': 3306,
+        'postgresql': 5432,
+        'mongodb': 27017,
+        'redis': 6379
+      };
+      
+      const scheme = parsedUrl.protocol.replace(':', '').toLowerCase();
+      port = defaultPorts[scheme] || 80;
+    }
+    
     const hostname = parsedUrl.hostname;
     
     // Check common ports first