Merge branch 'main' into emmercm/20230915-issue-templates

.github/labels.yml

@@ -2,7 +2,7 @@
 
 # GitHub defaults
 - name: bug
-  description: Something isn't working
+  description: A confirmed issues that needs fixing
   color: 'd73a4a'
 #- name: dependencies
 #  description: Pull requests that update a dependency file
@@ -25,9 +25,9 @@
 #- name: invalid
 #  description: This doesn't seem right
 #  color: 'e4e669'
-#- name: question
-#  description: Further information is requested
-#  color: 'd876e3'
+- name: question
+  description: Further information is requested
+  color: 'd876e3'
 #- name: wontfix
 #  description: This will not be worked on
 #  color: 'ffffff'
@@ -37,6 +37,14 @@
 #  color: '4c2e9c'
 #  alias: ['enhancement']
 - name: potential-bug
-  color: '66b9cc'
+  description: A potential issue that needs confirmation and/or triage
+  color: '9f06e2'
 - name: security
   color: 'f72e32'
+- name: up-for-grabs
+  description: Issues to consider for external contribution
+  color: '66cc8d'
+
+- name: stale
+  description: Issue has had no recent activity and will be automatically closed
+  color: 'ffffff'

.github/workflows/gh-stale-issues.yml

@@ -0,0 +1,20 @@
+name: GitHub Stale Issues
+
+on:
+  schedule:
+    - cron: '30 1 * * *'
+
+jobs:
+  stale:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/stale@v8
+        with:
+          days-before-issue-stale: 14
+          days-before-pr-stale: -1
+          days-before-close: 14
+          stale-issue-message: 'This issue is stale because it has been open 14 days with no activity. Comment or this will be automatically closed in 14 days.'
+          close-issue-message: 'This issue was closed because it has been stalled for 14 days with no activity. Only repository collaborators will be able to re-open this issue.'
+          stale-issue-label: 'stale'
+          only-labels: 'question'
+          exempt-assignees: 'emmercm'

src/igir.ts

@@ -78,7 +78,7 @@ export default class Igir {
       throw new Error('No DAT contains parent/clone information, cannot process --single');
     }
 
-    const datsToWrittenRoms = new Map<DAT, Map<Parent, File[]>>();
+    const datsToWrittenFiles = new Map<DAT, File[]>();
     const romOutputDirs: string[] = [];
     const movedRomsToDelete: File[] = [];
     const datsStatuses: DATStatus[] = [];
@@ -109,20 +109,22 @@ export default class Igir {
       const movedRoms = await new CandidateWriter(this.options, progressBar)
         .write(filteredDat, parentsToCandidates);
       movedRomsToDelete.push(...movedRoms);
-      const writtenRoms = [...parentsToCandidates.entries()]
-        .reduce((map, [parent, releaseCandidates]) => {
-          // For each Parent, find what rom Files were written
-          const parentWrittenRoms = releaseCandidates
-            .flatMap((releaseCandidate) => releaseCandidate.getRomsWithFiles())
-            .map((romWithFiles) => romWithFiles.getOutputFile());
-          map.set(parent, parentWrittenRoms);
-          return map;
-        }, new Map<Parent, File[]>());
-      datsToWrittenRoms.set(filteredDat, writtenRoms);
+      const writtenRoms = [...parentsToCandidates.values()]
+        .flatMap((releaseCandidates) => releaseCandidates)
+        .flatMap((releaseCandidate) => releaseCandidate
+          .getRomsWithFiles()
+          .map((romWithFiles) => romWithFiles.getOutputFile()));
+      datsToWrittenFiles.set(filteredDat, writtenRoms);
 
       // Write a fixdat
-      await new FixdatCreator(this.options, progressBar)
+      const fixdatPath = await new FixdatCreator(this.options, progressBar)
         .write(filteredDat, parentsToCandidates);
+      if (fixdatPath) {
+        datsToWrittenFiles.set(filteredDat, [
+          ...(datsToWrittenFiles.get(filteredDat) ?? []),
+          await File.fileOf(fixdatPath),
+        ]);
+      }
 
       // Write the output report
       const datStatus = await new StatusGenerator(this.options, progressBar)
@@ -147,10 +149,10 @@ export default class Igir {
     datProcessProgressBar.delete();
 
     // Delete moved ROMs
-    await this.deleteMovedRoms(roms, movedRomsToDelete, datsToWrittenRoms);
+    await this.deleteMovedRoms(roms, movedRomsToDelete, datsToWrittenFiles);
 
     // Clean the output directories
-    const cleanedOutputFiles = await this.processOutputCleaner(romOutputDirs, datsToWrittenRoms);
+    const cleanedOutputFiles = await this.processOutputCleaner(romOutputDirs, datsToWrittenFiles);
 
     // Generate the report
     await this.processReportGenerator(roms, cleanedOutputFiles, datsStatuses);
@@ -270,31 +272,30 @@ export default class Igir {
   private async deleteMovedRoms(
     rawRomFiles: File[],
     movedRomsToDelete: File[],
-    datsToWrittenRoms: Map<DAT, Map<Parent, File[]>>,
+    datsToWrittenFiles: Map<DAT, File[]>,
   ): Promise<void> {
     if (!movedRomsToDelete.length) {
       return;
     }
 
     const progressBar = await this.logger.addProgressBar('Deleting moved files');
     const deletedFilePaths = await new MovedROMDeleter(progressBar)
-      .delete(rawRomFiles, movedRomsToDelete, datsToWrittenRoms);
+      .delete(rawRomFiles, movedRomsToDelete, datsToWrittenFiles);
     await progressBar.doneItems(deletedFilePaths.length, 'moved file', 'deleted');
     await progressBar.freeze();
   }
 
   private async processOutputCleaner(
     dirsToClean: string[],
-    datsToWrittenRoms: Map<DAT, Map<Parent, File[]>>,
+    datsToWrittenFiles: Map<DAT, File[]>,
   ): Promise<string[]> {
     if (!this.options.shouldWrite() || !this.options.shouldClean()) {
       return [];
     }
 
     const progressBar = await this.logger.addProgressBar('Cleaning output directory');
     const uniqueDirsToClean = dirsToClean.reduce(ArrayPoly.reduceUnique(), []);
-    const writtenFilesToExclude = [...datsToWrittenRoms.values()]
-      .flatMap((parentsToFiles) => [...parentsToFiles.values()])
+    const writtenFilesToExclude = [...datsToWrittenFiles.values()]
       .flatMap((files) => files);
     const filesCleaned = await new DirectoryCleaner(this.options, progressBar)
       .clean(uniqueDirsToClean, writtenFilesToExclude);

src/modules/movedRomDeleter.ts

@@ -4,7 +4,6 @@ import fsPoly from '../polyfill/fsPoly.js';
 import ArchiveEntry from '../types/files/archives/archiveEntry.js';
 import File from '../types/files/file.js';
 import DAT from '../types/logiqx/dat.js';
-import Parent from '../types/logiqx/parent.js';
 import Module from './module.js';
 
 /**
@@ -25,7 +24,7 @@ export default class MovedROMDeleter extends Module {
   async delete(
     inputRoms: File[],
     movedRoms: File[],
-    datsToWrittenRoms: Map<DAT, Map<Parent, File[]>>,
+    datsToWrittenFiles: Map<DAT, File[]>,
   ): Promise<string[]> {
     if (!movedRoms.length) {
       return [];
@@ -39,7 +38,7 @@ export default class MovedROMDeleter extends Module {
 
     const filePathsToDelete = MovedROMDeleter.filterOutWrittenFiles(
       fullyConsumedFiles,
-      datsToWrittenRoms,
+      datsToWrittenFiles,
     );
 
     await this.progressBar.setSymbol(ProgressBarSymbol.DELETING);
@@ -128,10 +127,9 @@ export default class MovedROMDeleter extends Module {
    */
   private static filterOutWrittenFiles(
     movedRoms: string[],
-    datsToWrittenRoms: Map<DAT, Map<Parent, File[]>>,
+    datsToWrittenFiles: Map<DAT, File[]>,
   ): string[] {
-    const writtenFilePaths = new Set([...datsToWrittenRoms.values()]
-      .flatMap((parentsToFiles) => [...parentsToFiles.values()])
+    const writtenFilePaths = new Set([...datsToWrittenFiles.values()]
       .flatMap((files) => files)
       .map((file) => file.getFilePath()));
 

test/igir.test.ts

@@ -581,7 +581,7 @@ describe('with explicit DATs', () => {
   ])('should generate a fixdat when writing: %s', async (command) => {
     await copyFixturesToTemp(async (inputTemp, outputTemp) => {
       const result = await runIgir({
-        commands: [command],
+        commands: [command, 'clean'],
         dat: [path.join(inputTemp, 'dats')],
         input: [path.join(inputTemp, 'roms')],
         output: outputTemp,
@@ -691,6 +691,22 @@ describe('with inferred DATs', () => {
     });
   });
 
+  it('should move to the same directory', async () => {
+    await copyFixturesToTemp(async (inputTemp, outputTemp) => {
+      const inputDir = path.join(inputTemp, 'roms', 'raw');
+      const inputBefore = await walkWithCrc(inputDir, inputDir);
+
+      await runIgir({
+        commands: ['move', 'test'],
+        input: [inputDir],
+        output: inputDir,
+      });
+
+      await expect(walkWithCrc(inputDir, inputDir)).resolves.toEqual(inputBefore);
+      await expect(walkWithCrc(inputTemp, outputTemp)).resolves.toHaveLength(0);
+    });
+  });
+
   it('should move, extract, and test', async () => {
     await copyFixturesToTemp(async (inputTemp, outputTemp) => {
       const result = await runIgir({

test/modules/candidateWriter.test.ts

@@ -318,6 +318,18 @@ describe('zip', () => {
     });
   });
 
+  it('should not move if tested zip has wrong number of entries', () => {
+    // TODO(cemmer)
+  });
+
+  it('should not move if tested zip is missing an entry', () => {
+    // TODO(cemmer)
+  });
+
+  it('should not move if tested zip has an entry with an unexpected checksum', () => {
+    // TODO(cemmer)
+  });
+
   test.each([
     // Control group of un-headered files
     ['raw/empty.rom', 'empty.rom', '00000000'],

test/modules/movedRomDeleter.test.ts

@@ -153,14 +153,11 @@ describe('should delete archives', () => {
         .flatMap((releaseCandidate) => releaseCandidate.getRomsWithFiles())
         .map((romWithFiles) => romWithFiles.getInputFile());
 
-      const parentsToWrittenRoms = new Map([...parentsToCandidates.entries()]
-        .map(([parent, releaseCandidates]) => {
-          const writtenRoms = releaseCandidates
-            .flatMap((releaseCandidate) => releaseCandidate.getRomsWithFiles())
-            .map((romWithFiles) => romWithFiles.getOutputFile());
-          return [parent, writtenRoms];
-        }));
-      const datsToWrittenRoms = new Map([[dat, parentsToWrittenRoms]]);
+      const writtenRoms = [...parentsToCandidates.values()]
+        .flatMap((releaseCandidates) => releaseCandidates)
+        .flatMap((releaseCanddiate) => releaseCanddiate.getRomsWithFiles())
+        .map((romWithFiles) => romWithFiles.getOutputFile());
+      const datsToWrittenRoms = new Map([[dat, writtenRoms]]);
 
       const deletedFilePaths = (
         await new MovedROMDeleter(new ProgressBarFake())

test/polyfill/filePoly.test.ts

@@ -36,3 +36,31 @@ describe('fileOfSize', () => {
     }
   });
 });
+
+describe('readAt', () => {
+  it('should read a small file', async () => {
+    const tempFile = await fsPoly.mktemp(path.join(Constants.GLOBAL_TEMP_DIR, 'file'));
+    await expect(fsPoly.exists(tempFile)).resolves.toEqual(false);
+
+    try {
+      const file = await filePoly.fileOfSize(tempFile, 'r', Constants.MAX_MEMORY_FILE_SIZE - 1);
+      await expect(file.readAt(0, 16)).resolves.toEqual(Buffer.alloc(16));
+      await file.close();
+    } finally {
+      await fsPoly.rm(tempFile);
+    }
+  });
+
+  it('should read a large file', async () => {
+    const tempFile = await fsPoly.mktemp(path.join(Constants.GLOBAL_TEMP_DIR, 'file'));
+    await expect(fsPoly.exists(tempFile)).resolves.toEqual(false);
+
+    try {
+      const file = await filePoly.fileOfSize(tempFile, 'r', Constants.MAX_MEMORY_FILE_SIZE + 1);
+      await expect(file.readAt(0, 16)).resolves.toEqual(Buffer.alloc(16));
+      await file.close();
+    } finally {
+      await fsPoly.rm(tempFile);
+    }
+  });
+});

test/polyfill/fsPoly.test.ts

@@ -3,6 +3,79 @@ import path from 'path';
 import Constants from '../../src/constants.js';
 import fsPoly from '../../src/polyfill/fsPoly.js';
 
+describe('canSymlink', () => {
+  it('should not throw', async () => {
+    await expect(fsPoly.canSymlink(Constants.GLOBAL_TEMP_DIR)).resolves.not.toThrow();
+  });
+});
+
+describe('isDirectory', () => {
+  it('should return true for a directory', async () => {
+    const tempDir = await fsPoly.mkdtemp(Constants.GLOBAL_TEMP_DIR);
+    await expect(fsPoly.isDirectory(tempDir)).resolves.toEqual(true);
+  });
+
+  it('should return false for a file', async () => {
+    const tempFile = await fsPoly.mktemp(path.join(Constants.GLOBAL_TEMP_DIR, 'temp'));
+    await fsPoly.touch(tempFile);
+    await expect(fsPoly.isDirectory(tempFile)).resolves.toEqual(false);
+    await fsPoly.rm(tempFile);
+  });
+
+  it('should return false for non-existent file', async () => {
+    const tempFile = await fsPoly.mktemp(path.join(Constants.GLOBAL_TEMP_DIR, 'temp'));
+    await expect(fsPoly.isDirectory(tempFile)).resolves.toEqual(false);
+  });
+});
+
+describe('isDirectorySync', () => {
+  it('should return true for a directory', async () => {
+    const tempDir = await fsPoly.mkdtemp(Constants.GLOBAL_TEMP_DIR);
+    expect(fsPoly.isDirectorySync(tempDir)).toEqual(true);
+  });
+
+  it('should return false for a file', async () => {
+    const tempFile = await fsPoly.mktemp(path.join(Constants.GLOBAL_TEMP_DIR, 'temp'));
+    await fsPoly.touch(tempFile);
+    expect(fsPoly.isDirectorySync(tempFile)).toEqual(false);
+    await fsPoly.rm(tempFile);
+  });
+
+  it('should return false for non-existent file', async () => {
+    const tempFile = await fsPoly.mktemp(path.join(Constants.GLOBAL_TEMP_DIR, 'temp'));
+    expect(fsPoly.isDirectorySync(tempFile)).toEqual(false);
+  });
+});
+
+describe('isSymlink', () => {
+  it('should return true for a symlink', async () => {
+    const tempFile = await fsPoly.mktemp(path.join(Constants.GLOBAL_TEMP_DIR, 'temp'));
+    await fsPoly.touch(tempFile);
+    const tempLink = await fsPoly.mktemp(path.join(Constants.GLOBAL_TEMP_DIR, 'link'));
+    await fsPoly.symlink(tempFile, tempLink);
+    await expect(fsPoly.isSymlink(tempLink)).resolves.toEqual(true);
+    await fsPoly.rm(tempLink);
+    await fsPoly.rm(tempFile);
+  });
+
+  it('should return false for a plain directory', async () => {
+    const tempDir = await fsPoly.mkdtemp(Constants.GLOBAL_TEMP_DIR);
+    await expect(fsPoly.isSymlink(tempDir)).resolves.toEqual(false);
+  });
+
+  it('should return false for a plain file', async () => {
+    const tempFile = await fsPoly.mktemp(path.join(Constants.GLOBAL_TEMP_DIR, 'temp'));
+    await fsPoly.touch(tempFile);
+    await expect(fsPoly.isSymlink(tempFile)).resolves.toEqual(false);
+    await fsPoly.rm(tempFile);
+  });
+
+  it('should return false for non-existent file', async () => {
+    const tempFile = await fsPoly.mktemp(path.join(Constants.GLOBAL_TEMP_DIR, 'temp'));
+    await expect(fsPoly.isSymlink(tempFile)).resolves.toEqual(false);
+  });
+});
+
 describe('makeLegal', () => {
   describe('unix', () => {
     test.each([
@@ -70,6 +143,7 @@ describe('rm', () => {
     await fsPoly.rm(tempLink);
     await expect(fsPoly.exists(tempLink)).resolves.toEqual(false);
     await expect(fsPoly.exists(tempFile)).resolves.toEqual(true);
+    await fsPoly.rm(tempFile);
   });
 });
 
@@ -110,5 +184,6 @@ describe('rmSync', () => {
     fsPoly.rmSync(tempLink);
     await expect(fsPoly.exists(tempLink)).resolves.toEqual(false);
     await expect(fsPoly.exists(tempFile)).resolves.toEqual(true);
+    fsPoly.rmSync(tempFile);
   });
 });