Fix: don't throw if a single dat/input/patch directory is empty (#1231)

src/modules/datScanner.ts

@@ -18,6 +18,7 @@ import MameDAT from '../types/dats/mame/mameDat.js';
 import ROM from '../types/dats/rom.js';
 import SoftwareListDAT from '../types/dats/softwarelist/softwareListDat.js';
 import SoftwareListsDAT from '../types/dats/softwarelist/softwareListsDat.js';
+import ExpectedError from '../types/expectedError.js';
 import File from '../types/files/file.js';
 import { ChecksumBitmask } from '../types/files/fileChecksums.js';
 import Options from '../types/options.js';
@@ -96,8 +97,7 @@ export default class DATScanner extends Scanner {
           ChecksumBitmask.NONE,
         );
       } catch (error) {
-        this.progressBar.logError(`${datFile.toString()}: failed to download: ${error}`);
-        return [];
+        throw new ExpectedError(`failed to download '${datFile.toString()}': ${error}`);
       }
     }))).flat();
   }

src/types/cache.ts

@@ -182,7 +182,7 @@ export default class Cache<V> {
       if (this.maxSize !== undefined) {
         this.keyOrder = new Set(Object.keys(keyValuesObject));
       }
-    } catch { /* empty */ }
+    } catch { /* ignored */ }
 
     return this;
   }

src/types/files/archives/gzip.ts

@@ -22,7 +22,7 @@ export default class Gzip extends SevenZip {
     // See if this file is actually a .tar.gz
     try {
       return await new Tar(this.getFilePath()).getArchiveEntries(checksumBitmask);
-    } catch { /* empty */ }
+    } catch { /* ignored */ }
 
     return super.getArchiveEntries(checksumBitmask);
   }

src/types/options.ts

@@ -652,14 +652,11 @@ export default class Options implements OptionsProps {
     requireFiles = true,
   ): Promise<string[]> {
     // Limit to scanning one glob pattern at a time to keep memory in check
-    const uniqueGlobPatterns = globPatterns
-      .filter((pattern) => pattern)
-      .reduce(ArrayPoly.reduceUnique(), []);
+    const uniqueGlobPatterns = globPatterns.reduce(ArrayPoly.reduceUnique(), []);
     let globbedPaths: string[] = [];
     for (const uniqueGlobPattern of uniqueGlobPatterns) {
       const paths = await this.globPath(
         uniqueGlobPattern,
-        requireFiles,
         walkCallback ?? ((): void => {}),
       );
       // NOTE(cemmer): if `paths` is really large, `globbedPaths.push(...paths)` can hit a stack
@@ -689,14 +686,17 @@ export default class Options implements OptionsProps {
       .filter((inputPath, idx) => isNonDirectory[idx])
       .filter((inputPath) => isNotJunk(path.basename(inputPath)));
 
+    if (requireFiles && globbedFiles.length === 0) {
+      throw new ExpectedError(`no files found in director${globPatterns.length !== 1 ? 'ies' : 'y'}: ${globPatterns.map((p) => `'${p}'`).join(', ')}`);
+    }
+
     // Remove duplicates
     return globbedFiles
       .reduce(ArrayPoly.reduceUnique(), []);
   }
 
   private static async globPath(
     inputPath: string,
-    requireFiles: boolean,
     walkCallback: FsWalkCallback,
   ): Promise<string[]> {
     // Windows will report that \\.\nul doesn't exist, catch it explicitly
@@ -706,15 +706,8 @@ export default class Options implements OptionsProps {
 
     // Glob the contents of directories
     if (await fsPoly.isDirectory(inputPath)) {
-      const dirPaths = (await fsPoly.walk(inputPath, walkCallback))
+      return (await fsPoly.walk(inputPath, walkCallback))
         .map((filePath) => path.normalize(filePath));
-      if (dirPaths.length === 0) {
-        if (!requireFiles) {
-          return [];
-        }
-        throw new ExpectedError(`${inputPath}: directory doesn't contain any files`);
-      }
-      return dirPaths;
     }
 
     // If the file exists, don't process it as a glob pattern
@@ -728,6 +721,11 @@ export default class Options implements OptionsProps {
     // Try to handle globs a little more intelligently (see the JSDoc below)
     const inputPathEscaped = await this.sanitizeGlobPattern(inputPathNormalized);
 
+    if (!inputPathEscaped) {
+      // fast-glob will throw with empty-ish inputs
+      return [];
+    }
+
     // Otherwise, process it as a glob pattern
     const paths = (await fg(inputPathEscaped, { onlyFiles: true }))
       .map((filePath) => path.normalize(filePath));
@@ -737,11 +735,7 @@ export default class Options implements OptionsProps {
         walkCallback(1);
         return [inputPath];
       }
-
-      if (!requireFiles) {
-        return [];
-      }
-      throw new ExpectedError(`no files found in directory: ${inputPath}`);
+      return [];
     }
     walkCallback(paths.length);
     return paths;

test/igir.test.ts

@@ -137,22 +137,6 @@ async function runIgir(optionsProps: OptionsProps): Promise<TestOutput> {
 }
 
 describe('with explicit DATs', () => {
-  it('should do nothing with no roms', async () => {
-    await copyFixturesToTemp(async (inputTemp, outputTemp) => {
-      const result = await runIgir({
-        commands: ['copy'],
-        dat: [path.join(inputTemp, 'dats')],
-        input: [],
-        output: outputTemp,
-      });
-
-      expect(result.outputFilesAndCrcs).toHaveLength(0);
-      expect(result.cwdFilesAndCrcs).toHaveLength(0);
-      expect(result.movedFiles).toHaveLength(0);
-      expect(result.cleanedFiles).toHaveLength(0);
-    });
-  });
-
   it('should throw on all invalid dats', async () => {
     await expect(async () => new Igir(new Options({
       dat: ['src/*'],
@@ -854,21 +838,6 @@ describe('with explicit DATs', () => {
 });
 
 describe('with inferred DATs', () => {
-  it('should do nothing with no roms', async () => {
-    await copyFixturesToTemp(async (inputTemp, outputTemp) => {
-      const result = await runIgir({
-        commands: ['copy'],
-        input: [],
-        output: outputTemp,
-      });
-
-      expect(result.outputFilesAndCrcs).toHaveLength(0);
-      expect(result.cwdFilesAndCrcs).toHaveLength(0);
-      expect(result.movedFiles).toHaveLength(0);
-      expect(result.cleanedFiles).toHaveLength(0);
-    });
-  });
-
   it('should copy and test', async () => {
     await copyFixturesToTemp(async (inputTemp, outputTemp) => {
       const result = await runIgir({

test/modules/argumentsParser.test.ts

@@ -229,7 +229,7 @@ describe('options', () => {
     expect(() => argumentsParser.parse(['dir2dat', '--output', os.devNull])).toThrow(/missing required argument/i);
     expect(() => argumentsParser.parse(['fixdat', '--output', os.devNull])).toThrow(/missing required argument/i);
     await expect(argumentsParser.parse(['copy', '--input', 'nonexistentfile', '--output', os.devNull]).scanInputFilesWithoutExclusions()).rejects.toThrow(/no files found/i);
-    await expect(argumentsParser.parse(['copy', '--input', os.devNull, '--output', os.devNull]).scanInputFilesWithoutExclusions()).resolves.toHaveLength(0);
+    await expect(argumentsParser.parse(['copy', '--input', os.devNull, '--output', os.devNull]).scanInputFilesWithoutExclusions()).rejects.toThrow(/no files found/i);
 
     const src = await argumentsParser.parse(['copy', '--input', './src', '--output', os.devNull]).scanInputFilesWithoutExclusions();
     const test = await argumentsParser.parse(['copy', '--input', './test', '--output', os.devNull]).scanInputFilesWithoutExclusions();
@@ -274,9 +274,9 @@ describe('options', () => {
     expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dat'])).toThrow(/not enough arguments/i);
     expect(() => argumentsParser.parse([...dummyCommandAndRequiredArgs, 'dir2dat', '--dat', os.devNull])).toThrow();
     await expect(argumentsParser.parse(dummyCommandAndRequiredArgs).scanDatFilesWithoutExclusions())
-      .resolves.toHaveLength(0);
+      .rejects.toThrow(/no files found/i);
     await expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dat', 'nonexistentfile']).scanDatFilesWithoutExclusions()).rejects.toThrow(/no files found/i);
-    await expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dat', os.devNull]).scanDatFilesWithoutExclusions()).resolves.toHaveLength(0);
+    await expect(argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dat', os.devNull]).scanDatFilesWithoutExclusions()).rejects.toThrow(/no files found/i);
 
     const src = await argumentsParser.parse([...dummyCommandAndRequiredArgs, '-d', './src']).scanDatFilesWithoutExclusions();
     const test = await argumentsParser.parse([...dummyCommandAndRequiredArgs, '--dat', './test']).scanDatFilesWithoutExclusions();
@@ -412,7 +412,7 @@ describe('options', () => {
 
   it('should parse "patch"', async () => {
     await expect(argumentsParser.parse(['copy', '--input', os.devNull, '--patch', 'nonexistentfile', '--output', os.devNull]).scanPatchFilesWithoutExclusions()).rejects.toThrow(/no files found/i);
-    await expect(argumentsParser.parse(['copy', '--input', os.devNull, '--patch', os.devNull, '--output', os.devNull]).scanPatchFilesWithoutExclusions()).resolves.toHaveLength(0);
+    await expect(argumentsParser.parse(['copy', '--input', os.devNull, '--patch', os.devNull, '--output', os.devNull]).scanPatchFilesWithoutExclusions()).rejects.toThrow(/no files found/i);
 
     const src = await argumentsParser.parse(['copy', '--input', os.devNull, '--patch', './src', '--output', os.devNull]).scanPatchFilesWithoutExclusions();
     const test = await argumentsParser.parse(['copy', '--input', os.devNull, '--patch', './test', '--output', os.devNull]).scanPatchFilesWithoutExclusions();

test/modules/candidateWriter.test.ts

@@ -95,7 +95,12 @@ async function candidateWriter(
     ...(patchGlob ? { patch: [path.join(inputTemp, patchGlob)] } : {}),
     output: outputTemp,
   });
-  const romFiles = await new ROMScanner(options, new ProgressBarFake()).scan();
+
+  let romFiles: File[] = [];
+  try {
+    romFiles = await new ROMScanner(options, new ProgressBarFake()).scan();
+  } catch { /* ignored */ }
+
   const dat = datInferrer(options, romFiles);
   const romFilesWithHeaders = await new ROMHeaderProcessor(options, new ProgressBarFake())
     .process(romFiles);

test/modules/datGameInferrer.test.ts

@@ -4,8 +4,6 @@ import Options from '../../src/types/options.js';
 import ProgressBarFake from '../console/progressBarFake.js';
 
 test.each([
-  // No input paths
-  [[], {}],
   // One input path
   [['test/fixtures/roms/**/*'], { roms: 28 }],
   [['test/fixtures/roms/7z/*'], { '7z': 5 }],

test/modules/datScanner.test.ts

@@ -17,7 +17,6 @@ function createDatScanner(props: OptionsProps): DATScanner {
 test.each([
   [['/completely/invalid/path']],
   [['/completely/invalid/path', os.devNull]],
-  [['/completely/invalid/path', 'test/fixtures/dats']],
   [['test/fixtures/**/*.tmp']],
   [['test/fixtures/dats/*foo*/*bar*']],
 ])('should throw on nonexistent paths: %s', async (dat) => {
@@ -28,10 +27,15 @@ test.each([
   [[]],
   [['']],
   [[os.devNull]],
+])('should throw on no results: %s', async (dat) => {
+  await expect(createDatScanner({ dat }).scan()).rejects.toThrow(/no files found/i);
+});
+
+test.each([
   [['http://completelybadurl']],
   [['https://completelybadurl']],
-])('should return empty list on no results: %s', async (dat) => {
-  await expect(createDatScanner({ dat }).scan()).resolves.toHaveLength(0);
+])('should throw on bad URLs: %s', async (dat) => {
+  await expect(createDatScanner({ dat }).scan()).rejects.toThrow(/failed to download/i);
 });
 
 test.each([

test/modules/patchScanner.test.ts

@@ -15,15 +15,14 @@ function createPatchScanner(patch: string[], patchExclude: string[] = []): Patch
 it('should throw on nonexistent paths', async () => {
   await expect(createPatchScanner(['/completely/invalid/path']).scan()).rejects.toThrow(/no files found/i);
   await expect(createPatchScanner(['/completely/invalid/path', os.devNull]).scan()).rejects.toThrow(/no files found/i);
-  await expect(createPatchScanner(['/completely/invalid/path', 'test/fixtures/roms']).scan()).rejects.toThrow(/no files found/i);
   await expect(createPatchScanner(['test/fixtures/**/*.tmp']).scan()).rejects.toThrow(/no files found/i);
   await expect(createPatchScanner(['test/fixtures/roms/*foo*/*bar*']).scan()).rejects.toThrow(/no files found/i);
 });
 
-it('should return empty list on no results', async () => {
-  await expect(createPatchScanner([]).scan()).resolves.toHaveLength(0);
-  await expect(createPatchScanner(['']).scan()).resolves.toHaveLength(0);
-  await expect(createPatchScanner([os.devNull]).scan()).resolves.toHaveLength(0);
+it('should throw on no results', async () => {
+  await expect(createPatchScanner([]).scan()).rejects.toThrow(/no files found/i);
+  await expect(createPatchScanner(['']).scan()).rejects.toThrow(/no files found/i);
+  await expect(createPatchScanner([os.devNull]).scan()).rejects.toThrow(/no files found/i);
 });
 
 it('should return empty list on non-patches', async () => {

test/modules/romScanner.test.ts

@@ -15,15 +15,14 @@ function createRomScanner(input: string[], inputExclude: string[] = []): ROMScan
 it('should throw on nonexistent paths', async () => {
   await expect(createRomScanner(['/completely/invalid/path']).scan()).rejects.toThrow(/no files found/i);
   await expect(createRomScanner(['/completely/invalid/path', os.devNull]).scan()).rejects.toThrow(/no files found/i);
-  await expect(createRomScanner(['/completely/invalid/path', 'test/fixtures/roms']).scan()).rejects.toThrow(/no files found/i);
   await expect(createRomScanner(['test/fixtures/**/*.tmp']).scan()).rejects.toThrow(/no files found/i);
   await expect(createRomScanner(['test/fixtures/roms/*foo*/*bar*']).scan()).rejects.toThrow(/no files found/i);
 });
 
-it('should return empty list on no results', async () => {
-  await expect(createRomScanner([]).scan()).resolves.toHaveLength(0);
-  await expect(createRomScanner(['']).scan()).resolves.toHaveLength(0);
-  await expect(createRomScanner([os.devNull]).scan()).resolves.toHaveLength(0);
+it('should throw on no results', async () => {
+  await expect(createRomScanner([]).scan()).rejects.toThrow(/no files found/i);
+  await expect(createRomScanner(['']).scan()).rejects.toThrow(/no files found/i);
+  await expect(createRomScanner([os.devNull]).scan()).rejects.toThrow(/no files found/i);
 });
 
 it('should not throw on bad archives', async () => {