fix: Add proper error message for Odiff with S3 (#332)

* fix: Add proper error message for Odiff with S3

closes #327
closes Visual-Regression-Tracker/Visual-Regression-Tracker#458

* styling

Author: pashidlos

docker-compose.yml

@@ -6,10 +6,16 @@ services:
       dockerfile: Dockerfile
     environment:
       DATABASE_URL: postgresql://${POSTGRES_USER}:${POSTGRES_PASSWORD}@postgres:5432/${POSTGRES_DB}
+      STATIC_SERVICE: ${STATIC_SERVICE}
+      AWS_ACCESS_KEY_ID: ${AWS_ACCESS_KEY_ID}
+      AWS_SECRET_ACCESS_KEY: ${AWS_SECRET_ACCESS_KEY}
+      AWS_REGION: ${AWS_REGION}
+      AWS_S3_BUCKET_NAME: ${AWS_S3_BUCKET_NAME}
       JWT_SECRET: ${JWT_SECRET}
       JWT_LIFE_TIME: ${JWT_LIFE_TIME}
-      BODY_PARSER_JSON_LIMIT: ${BODY_PARSER_JSON_LIMIT}
       APP_FRONTEND_URL: ${APP_FRONTEND_URL}
+      BODY_PARSER_JSON_LIMIT: ${BODY_PARSER_JSON_LIMIT}
+      ELASTIC_URL: ${ELASTIC_URL}
     ports:
       - "${APP_PORT}:3000"
     expose:

src/compare/compare.service.spec.ts

@@ -5,9 +5,14 @@ import { LookSameService } from './libs/looks-same/looks-same.service';
 import { OdiffService } from './libs/odiff/odiff.service';
 import { PixelmatchService } from './libs/pixelmatch/pixelmatch.service';
 import { StaticModule } from '../static/static.module';
+import { ImageComparison } from '@prisma/client';
+import * as utils from '../static/utils';
 
 describe('CompareService', () => {
   let service: CompareService;
+  let pixelmatchService: PixelmatchService;
+  let lookSameService: LookSameService;
+  let odiffService: OdiffService;
 
   beforeEach(async () => {
     const module: TestingModule = await Test.createTestingModule({
@@ -16,9 +21,43 @@ describe('CompareService', () => {
     }).compile();
 
     service = module.get<CompareService>(CompareService);
+    pixelmatchService = module.get<PixelmatchService>(PixelmatchService);
+    lookSameService = module.get<LookSameService>(LookSameService);
+    odiffService = module.get<OdiffService>(OdiffService);
   });
 
   it('should be defined', () => {
     expect(service).toBeDefined();
   });
+
+  describe('getComparator', () => {
+    it('should return pixelmatchService', () => {
+      const result = service.getComparator(ImageComparison.pixelmatch);
+      expect(result).toBe(pixelmatchService);
+    });
+
+    it('should return lookSameService', () => {
+      const result = service.getComparator(ImageComparison.lookSame);
+      expect(result).toBe(lookSameService);
+    });
+
+    it('should return odiffService', () => {
+      jest.spyOn(utils, 'isHddStaticServiceConfigured').mockReturnValue(true);
+
+      expect(service.getComparator(ImageComparison.odiff)).toBe(odiffService);
+    });
+
+    it('should throw if not HDD for Odiff', () => {
+      jest.spyOn(utils, 'isHddStaticServiceConfigured').mockReturnValue(false);
+
+      expect(() => service.getComparator(ImageComparison.odiff)).toThrow(
+        'Odiff can only be used with HDD static service. Please use another image comparison lib in project settings or switch STATIC_SERVICE envitonmental variable to HDD.'
+      );
+    });
+
+    it('should return pixelmatchService for unknown value', () => {
+      const result = service.getComparator('unknown' as ImageComparison);
+      expect(result).toBe(pixelmatchService);
+    });
+  });
 });

src/compare/compare.service.ts

@@ -1,20 +1,23 @@
 import { ImageComparison, Project } from '@prisma/client';
-import { Injectable } from '@nestjs/common';
+import { Injectable, Logger } from '@nestjs/common';
 import { PixelmatchService } from './libs/pixelmatch/pixelmatch.service';
 import { ImageComparator } from './libs/image-comparator.interface';
 import { ImageCompareInput } from './libs/ImageCompareInput';
 import { PrismaService } from '../prisma/prisma.service';
 import { DiffResult } from '../test-runs/diffResult';
 import { LookSameService } from './libs/looks-same/looks-same.service';
 import { OdiffService } from './libs/odiff/odiff.service';
+import { isHddStaticServiceConfigured } from '../static/utils';
 
 @Injectable()
 export class CompareService {
+  private readonly logger: Logger = new Logger(CompareService.name);
+
   constructor(
-    private pixelmatchService: PixelmatchService,
-    private lookSameService: LookSameService,
-    private odiffService: OdiffService,
-    private prismaService: PrismaService
+    private readonly pixelmatchService: PixelmatchService,
+    private readonly lookSameService: LookSameService,
+    private readonly odiffService: OdiffService,
+    private readonly prismaService: PrismaService
   ) {}
 
   async getDiff({ projectId, data }: { projectId: string; data: ImageCompareInput }): Promise<DiffResult> {
@@ -33,9 +36,16 @@ export class CompareService {
         return this.lookSameService;
       }
       case ImageComparison.odiff: {
+        if (!isHddStaticServiceConfigured()) {
+          throw new Error(
+            'Odiff can only be used with HDD static service. Please use another image comparison lib in project settings or switch STATIC_SERVICE envitonmental variable to HDD.'
+          );
+        }
+
         return this.odiffService;
       }
       default: {
+        this.logger.warn(`Unknown ImageComparison value: ${imageComparison}. Falling back to pixelmatch.`);
         return this.pixelmatchService;
       }
     }

src/compare/libs/looks-same/looks-same.service.ts

@@ -21,7 +21,7 @@ export const DEFAULT_CONFIG: LooksSameConfig = {
 export class LookSameService implements ImageComparator {
   private readonly logger: Logger = new Logger(LookSameService.name);
 
-  constructor(private staticService: StaticService) {}
+  constructor(private readonly staticService: StaticService) {}
 
   parseConfig(configJson: string): LooksSameConfig {
     return parseConfig(configJson, DEFAULT_CONFIG, this.logger);

src/compare/libs/odiff/odiff.service.ts

@@ -10,7 +10,6 @@ import { compare } from 'odiff-bin';
 import { IgnoreAreaDto } from 'src/test-runs/dto/ignore-area.dto';
 import { OdiffConfig, OdiffIgnoreRegions, OdiffResult } from './odiff.types';
 import { HddService } from 'src/static/hdd/hdd.service';
-import { isHddStaticServiceConfigured } from '../../../static/utils';
 
 export const DEFAULT_CONFIG: OdiffConfig = {
   outputDiffMask: true,
@@ -24,12 +23,7 @@ export class OdiffService implements ImageComparator {
   private readonly logger: Logger = new Logger(OdiffService.name);
   private readonly hddService: HddService;
 
-  constructor(private staticService: StaticService) {
-    if (!isHddStaticServiceConfigured()) {
-      return undefined;
-      // If we throw an exception, the application does not start.
-      // throw new Error('OdiffService can only be used with HddService');
-    }
+  constructor(private readonly staticService: StaticService) {
     this.hddService = this.staticService as unknown as HddService;
   }
 

src/compare/libs/pixelmatch/pixelmatch.service.ts

@@ -16,7 +16,7 @@ export const DEFAULT_CONFIG: PixelmatchConfig = { threshold: 0.1, ignoreAntialia
 export class PixelmatchService implements ImageComparator {
   private readonly logger: Logger = new Logger(PixelmatchService.name);
 
-  constructor(private staticService: StaticService) {}
+  constructor(private readonly staticService: StaticService) {}
 
   parseConfig(configJson: string): PixelmatchConfig {
     return parseConfig(configJson, DEFAULT_CONFIG, this.logger);