fix: extract base command, improve type safety, add additional errors handling, and improve tests coverage (#281)

* Initial plan

* Initial commit - planning code review improvements

Co-authored-by: panz3r <1754457+panz3r@users.noreply.github.com>

* Task 1: Extract base command class to eliminate code duplication

Co-authored-by: panz3r <1754457+panz3r@users.noreply.github.com>

* Task 2: Add type definitions for icon sizes

Co-authored-by: panz3r <1754457+panz3r@users.noreply.github.com>

* Task 3: Improve error collection and reporting in commands

Co-authored-by: panz3r <1754457+panz3r@users.noreply.github.com>

* Task 4: Add tests for malformed/corrupt image inputs

Co-authored-by: panz3r <1754457+panz3r@users.noreply.github.com>

* Task 5: Add unit tests for file-utils and color.utils

Co-authored-by: panz3r <1754457+panz3r@users.noreply.github.com>

* Fix linting issues and update CODE-REVIEW-FINDINGS.md to mark all items complete

Co-authored-by: panz3r <1754457+panz3r@users.noreply.github.com>

* Remove package-lock.json and add it to .gitignore (pnpm project)

Co-authored-by: panz3r <1754457+panz3r@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: panz3r <1754457+panz3r@users.noreply.github.com>

Author: Copilot

.gitignore

@@ -234,3 +234,6 @@ oclif.manifest.json
 
 # TypeScript build info
 *.tsbuildinfo
+
+# npm lock file (this is a pnpm project)
+package-lock.json

docs/001-CODE-REVIEW-FINDINGS.md

@@ -1,27 +1,28 @@
 # Code Review Findings
 
-**Status:** Partially Completed  
+**Status:** ✅ Completed  
 **Created:** 2026-01-05  
 **Updated:** 2026-01-06  
 **Target:** Improvement recommendations from a code review, prioritized for the "Test Developer" agent.
 
 ## Completion Summary
 
-**Completed Items (6/11):**
+**Completed Items (11/11):**
 - ✅ Fix typo in error messages (icons.ts, splash.ts)
 - ✅ Add missing license header to constants.ts
 - ✅ Standardize path prefixes in splash.ts
 - ✅ Improve extractAppName error handling
 - ✅ Add verbose flag to dotenv command
 - ✅ Add tests for verbose output (icons, splash, dotenv)
 - ✅ Add unit tests for extractAppName utility
+- ✅ Add tests for malformed input images
+- ✅ Extract base command class
+- ✅ Add type definitions for icon sizes
+- ✅ Improve error collection in commands
+- ✅ Add tests for utility files (file-utils, color.utils)
 
-**Pending Items (5/11):**
-- ⏳ Add tests for malformed input images
-- ⏳ Extract base command class
-- ⏳ Add type definitions for icon sizes
-- ⏳ Improve error collection in commands
-- ⏳ Add tests for utility files (file-utils, color.utils)
+**Pending Items (0/11):**
+- None - all items completed!
 
 ---
 
@@ -48,23 +49,29 @@ it('runs icons with verbose flag and shows detailed output', async () => {
 })
 ```
 
-### 2. Add Tests for Malformed Input Images ⏳ PENDING
+### 2. Add Tests for Malformed Input Images ✅ COMPLETED
 
-**Status:** ⏳ Not yet implemented
+**Status:** ✅ Completed in this PR
 
-No tests verify behavior with corrupt or wrong-format files.
+Tests now verify behavior with corrupt or wrong-format files.
 
-**Test cases to add:**
+**Test cases added:**
 ```typescript
 it('handles corrupt image file gracefully', async () => {
-  fs.writeFileSync('assets/icon.png', 'not a valid image')
+  const corruptFile = 'assets/corrupt-icon.png'
+  fs.writeFileSync(corruptFile, 'not a valid image')
 
-  const {error} = await runCommand(['icons', '--appName', 'test'])
+  const {stdout} = await runCommand(['icons', '--appName', 'TestApp', corruptFile])
 
-  // Verify appropriate error handling
+  // Should handle error gracefully - verify error collection message appears
+  expect(stdout).to.match(/failed to generate|asset.*failed/i)
 })
 ```
 
+**Files updated:**
+- ✅ `test/commands/icons.test.ts` - Added corrupt image test
+- ✅ `test/commands/splash.test.ts` - Added corrupt image test
+
 ### 3. Add Unit Tests for `extractAppName` Utility ✅ COMPLETED
 
 **Status:** ✅ Completed in PR #XX
@@ -245,13 +252,13 @@ export function extractAppName(): string | null {
 
 ## Refactoring Suggestions (Lower Priority)
 
-### 1. Extract Base Command Class ⏳ PENDING
+### 1. Extract Base Command Class ✅ COMPLETED
 
-**Status:** ⏳ Not yet implemented (Priority: Medium)
+**Status:** ✅ Completed in this PR (Priority: Medium)
 
 **Issue:** Both `Icons` and `Splash` commands duplicate the same `logVerbose()` implementation and `_isVerbose` property.
 
-**File to create:** `src/commands/base.ts`
+**File created:** `src/commands/base.ts`
 
 ```typescript
 /*
@@ -275,15 +282,17 @@ export abstract class BaseCommand extends Command {
 }
 ```
 
-**Then update commands to extend `BaseCommand` instead of `Command`.**
+**Files updated:**
+- ✅ `src/commands/icons.ts` - Now extends `BaseCommand` instead of `Command`
+- ✅ `src/commands/splash.ts` - Now extends `BaseCommand` instead of `Command`
 
-### 2. Add Type Definitions for Icon Sizes ⏳ PENDING
+### 2. Add Type Definitions for Icon Sizes ✅ COMPLETED
 
-**Status:** ⏳ Not yet implemented (Priority: Low)
+**Status:** ✅ Completed in this PR (Priority: Low)
 
 **File:** `src/types.ts`
 
-Add interfaces for icon sizes (currently only splashscreen sizes are typed):
+Added interfaces for icon sizes (previously only splashscreen sizes were typed):
 
 ```typescript
 export interface IconSizeAndroid {
@@ -299,21 +308,21 @@ export interface IconSizeIOS {
 }
 ```
 
-**Then update `src/constants.ts`:**
+**Updated `src/constants.ts`:**
 ```typescript
 export const ICON_SIZES_ANDROID: Array<IconSizeAndroid> = [...]
 export const ICON_SIZES_IOS: Array<IconSizeIOS> = [...]
 ```
 
-### 3. Improve Error Collection ⏳ PENDING
+### 3. Improve Error Collection ✅ COMPLETED
 
-**Status:** ⏳ Not yet implemented (Priority: Medium)
+**Status:** ✅ Completed in this PR (Priority: Medium)
 
 **Issue:** In `icons.ts` and `splash.ts`, errors during image generation are logged but execution continues silently. Users may not realize some icons failed to generate.
 
 **Files:** `src/commands/icons.ts`, `src/commands/splash.ts`
 
-**Suggestion:** Collect errors and report summary at end:
+**Implementation:** Errors are now collected and reported as a summary at the end:
 
 ```typescript
 private errors: string[] = []
@@ -324,7 +333,9 @@ this.errors.push(`Failed to generate: ${outputPath}`)
 // At end of run():
 if (this.errors.length > 0) {
   this.warn(`${yellow('⚠')} ${this.errors.length} asset(s) failed to generate:`)
-  this.errors.forEach(err => this.log(`  - ${err}`))
+  for (const err of this.errors) {
+    this.log(`  - ${err}`)
+  }
 }
 ```
 
@@ -334,22 +345,23 @@ if (this.errors.length > 0) {
 
 | File | Test File | Status |
 |------|-----------|--------|
-| `src/commands/icons.ts` | `test/commands/icons.test.ts` | ✅ Verbose tests added, typo fixed |
-| `src/commands/splash.ts` | `test/commands/splash.test.ts` | ✅ Verbose tests added, typo fixed, paths standardized |
+| `src/commands/icons.ts` | `test/commands/icons.test.ts` | ✅ Verbose tests added, typo fixed, base class extracted, error collection added, corrupt image test added |
+| `src/commands/splash.ts` | `test/commands/splash.test.ts` | ✅ Verbose tests added, typo fixed, paths standardized, base class extracted, error collection added, corrupt image test added |
 | `src/commands/dotenv.ts` | `test/commands/dotenv.test.ts` | ✅ Verbose flag + tests added |
+| `src/commands/base.ts` | N/A | ✅ Base command class created |
 | `src/utils/app.utils.ts` | `test/app.utils.test.ts` | ✅ Test file created, error handling improved |
-| `src/utils/file-utils.ts` | ❌ Missing | ⏳ Consider adding tests |
-| `src/utils/color.utils.ts` | ❌ Missing | ⏳ Consider adding tests |
-| `src/constants.ts` | N/A | ✅ License header added |
-| `src/types.ts` | N/A | ⏳ Icon size interfaces pending |
+| `src/utils/file-utils.ts` | `test/utils/file-utils.test.ts` | ✅ Test file created with comprehensive coverage |
+| `src/utils/color.utils.ts` | `test/utils/color.utils.test.ts` | ✅ Test file created with comprehensive coverage |
+| `src/constants.ts` | N/A | ✅ License header added, type annotations added |
+| `src/types.ts` | N/A | ✅ Icon size interfaces added |
 
 ---
 
 ## Priority Summary
 
 | Priority | Completed | Pending | Items |
 |----------|-----------|---------|-------|
-| 🔶 Medium | 2 | 3 | ✅ Verbose tests, dotenv verbose flag ⏳ Base command extraction, error collection, malformed input tests |
-| 🔷 Low | 4 | 2 | ✅ Typo fix, license header, path prefixes, extractAppName improvement ⏳ Icon size types, utility tests |
+| 🔶 Medium | 5 | 0 | ✅ Verbose tests, dotenv verbose flag, base command extraction, error collection, malformed input tests |
+| 🔷 Low | 6 | 0 | ✅ Typo fix, license header, path prefixes, extractAppName improvement, icon size types, utility tests |
 
-**Overall Progress:** 6/11 items completed (55%)
+**Overall Progress:** 11/11 items completed (100%) ✅

src/commands/base.ts

@@ -0,0 +1,19 @@
+/*
+ * Copyright (c) 2025 ForWarD Software (https://forwardsoftware.solutions/)
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+import { Command } from '@oclif/core'
+
+export abstract class BaseCommand extends Command {
+  protected _isVerbose: boolean = false
+
+  protected logVerbose(message?: string, ...args: unknown[]) {
+    if (this._isVerbose) {
+      this.log(message, ...args)
+    }
+  }
+}

src/commands/icons.ts

@@ -6,7 +6,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-import { Args, Command, Flags } from '@oclif/core'
+import { Args, Flags } from '@oclif/core'
 import { writeFile } from 'node:fs/promises'
 import { join } from 'node:path'
 import sharp from 'sharp'
@@ -18,8 +18,9 @@ import { MaskType } from '../types.js'
 import { extractAppName } from '../utils/app.utils.js'
 import { cyan, green, red, yellow } from '../utils/color.utils.js'
 import { checkAssetFile, mkdirp } from '../utils/file-utils.js'
+import { BaseCommand } from './base.js'
 
-export default class Icons extends Command {
+export default class Icons extends BaseCommand {
   static override args = {
     file: Args.string({
       default: './assets/icon.png',
@@ -49,7 +50,7 @@ The template icon file should be at least 1024x1024px.
       description: 'Print more detailed log messages.',
     }),
   }
-  private _isVerbose: boolean = false
+  private errors: string[] = []
 
   public async run(): Promise<void> {
     const { args, flags } = await this.parse(Icons)
@@ -73,6 +74,13 @@ The template icon file should be at least 1024x1024px.
       this.generateIOSIcons(args.file, `ios/${flags.appName}/Images.xcassets/AppIcon.appiconset`),
     ])
 
+    if (this.errors.length > 0) {
+      this.warn(`${yellow('⚠')} ${this.errors.length} asset(s) failed to generate:`)
+      for (const err of this.errors) {
+        this.log(`  - ${err}`)
+      }
+    }
+
     this.log(green('✔'), `Generated icons for '${cyan(flags.appName)}' app.`)
   }
 
@@ -93,6 +101,7 @@ The template icon file should be at least 1024x1024px.
 
       this.logVerbose(green('✔'), cyan('Android'), `Icon '${cyan(outputPath)}' generated.`)
     } catch (error) {
+      this.errors.push(`Failed to generate: ${outputPath}`)
       this.log(red('✘'), cyan('Android'), `Failed to generate icon '${cyan(outputPath)}':`, error)
     }
   }
@@ -148,6 +157,7 @@ The template icon file should be at least 1024x1024px.
 
       this.logVerbose(green('✔'), cyan('iOS'), `Icon '${cyan(filename)}' generated.`)
     } catch (error) {
+      this.errors.push(`Failed to generate: ${filename}`)
       this.log(red('✘'), cyan('iOS'), `Failed to generate icon '${cyan(filename)}'.`, error)
     }
   }
@@ -204,10 +214,4 @@ The template icon file should be at least 1024x1024px.
     const radius = Math.floor(size / 2)
     return Buffer.from(`<svg><circle cx="${radius}" cy="${radius}" r="${radius}" /></svg>`)
   }
-
-  private logVerbose(message?: string, ...args: unknown[]) {
-    if (this._isVerbose) {
-      this.log(message, ...args)
-    }
-  }
 }

src/commands/splash.ts

@@ -6,7 +6,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-import { Args, Command, Flags } from '@oclif/core'
+import { Args, Flags } from '@oclif/core'
 import { writeFile } from 'node:fs/promises'
 import { join } from 'node:path'
 import sharp from 'sharp'
@@ -17,8 +17,9 @@ import { SPLASHSCREEN_SIZES_ANDROID, SPLASHSCREEN_SIZES_IOS } from '../constants
 import { extractAppName } from '../utils/app.utils.js'
 import { cyan, green, red, yellow } from '../utils/color.utils.js'
 import { checkAssetFile, mkdirp } from '../utils/file-utils.js'
+import { BaseCommand } from './base.js'
 
-export default class Splash extends Command {
+export default class Splash extends BaseCommand {
   static override args = {
     file: Args.string({
       default: './assets/splashscreen.png',
@@ -48,7 +49,7 @@ The template splashscreen file should be at least 1242x2208px.
       description: 'Print more detailed log messages.',
     }),
   }
-  private _isVerbose: boolean = false
+  private errors: string[] = []
 
   public async run(): Promise<void> {
     const { args, flags } = await this.parse(Splash)
@@ -72,6 +73,13 @@ The template splashscreen file should be at least 1242x2208px.
       this.generateIOSSplashscreens(args.file, `ios/${flags.appName}/Images.xcassets/Splashscreen.imageset`),
     ])
 
+    if (this.errors.length > 0) {
+      this.warn(`${yellow('⚠')} ${this.errors.length} asset(s) failed to generate:`)
+      for (const err of this.errors) {
+        this.log(`  - ${err}`)
+      }
+    }
+
     this.log(green('✔'), `Generated splashscreens for '${cyan(flags.appName)}' app.`)
   }
 
@@ -154,17 +162,12 @@ The template splashscreen file should be at least 1242x2208px.
 
       this.logVerbose(green('✔'), `Splashscreen '${cyan(outputPath)}' generated.`)
     } catch (error) {
+      this.errors.push(`Failed to generate: ${outputPath}`)
       this.log(red('✘'), `Failed to generate splashscreen '${cyan(outputPath)}':`, error)
     }
   }
 
   private getIOSAssetNameForDensity(density?: string): string {
     return `splashscreen${density ? `@${density}` : ''}.png`
   }
-
-  private logVerbose(message?: string, ...args: unknown[]) {
-    if (this._isVerbose) {
-      this.log(message, ...args)
-    }
-  }
 }

src/constants.ts

@@ -6,13 +6,13 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-import type { SplashscreenSize } from "./types.js"
+import type { IconSizeAndroid, IconSizeIOS, SplashscreenSize } from "./types.js"
 
 /**
  * Android Assets Sizes
  */
 
-export const ICON_SIZES_ANDROID = [
+export const ICON_SIZES_ANDROID: Array<IconSizeAndroid> = [
   {
     density: 'mdpi',
     size: 48,
@@ -72,7 +72,7 @@ export const SPLASHSCREEN_SIZES_ANDROID: Array<SplashscreenSize> = [
  * iOS Assets Sizes
  */
 
-export const ICON_SIZES_IOS = [
+export const ICON_SIZES_IOS: Array<IconSizeIOS> = [
   {
     baseSize: 20,
     name: 'Icon-Notification',

src/types.ts

@@ -4,6 +4,18 @@ export interface SplashscreenSize {
   width: number;
 }
 
+export interface IconSizeAndroid {
+  density: string;
+  size: number;
+}
+
+export interface IconSizeIOS {
+  baseSize: number;
+  idiom?: string;
+  name: string;
+  scales: number[];
+}
+
 export interface ContentJsonImage {
   filename: string;
   idiom: string;