From 8fcaacad52ea0317379b25ceecfa9678f45a3237 Mon Sep 17 00:00:00 2001 From: Christian Emmer <10749361+emmercm@users.noreply.github.com> Date: Tue, 6 Aug 2024 16:01:58 -0700 Subject: [PATCH] CI: increase test coverage --- .github/workflows/node-test.yml | 2 + src/modules/candidateArchiveFileHasher.ts | 5 ++- src/modules/candidateGenerator.ts | 19 +--------- src/modules/datGameInferrer.ts | 20 +++++----- src/modules/fixdatCreator.ts | 2 +- test/igir.test.ts | 24 ++++++++---- test/polyfill/fsPoly.test.ts | 46 +++++++++++++++++++++++ 7 files changed, 81 insertions(+), 37 deletions(-) diff --git a/.github/workflows/node-test.yml b/.github/workflows/node-test.yml index 66e3abc03..16573d494 100644 --- a/.github/workflows/node-test.yml +++ b/.github/workflows/node-test.yml @@ -69,6 +69,7 @@ jobs: - path-filter if: ${{ needs.path-filter.outputs.changes == 'true' }} runs-on: ${{ matrix.os }}-latest + timeout-minutes: 10 strategy: matrix: os: [ ubuntu, macos, windows ] @@ -97,6 +98,7 @@ jobs: - path-filter if: ${{ needs.path-filter.outputs.changes == 'true' }} runs-on: ubuntu-latest + timeout-minutes: 10 strategy: matrix: node-version: [ lts, 18, 16.7.0 ] diff --git a/src/modules/candidateArchiveFileHasher.ts b/src/modules/candidateArchiveFileHasher.ts index 56b13cc2e..72531f60e 100644 --- a/src/modules/candidateArchiveFileHasher.ts +++ b/src/modules/candidateArchiveFileHasher.ts @@ -83,7 +83,8 @@ export default class CandidateArchiveFileHasher extends Module { return romWithFiles; } - if (inputFile.equals(romWithFiles.getOutputFile())) { + const outputFile = romWithFiles.getOutputFile(); + if (inputFile.equals(outputFile)) { // There's no need to calculate the checksum, {@link CandidateWriter} will skip // writing over itself return romWithFiles; @@ -103,7 +104,7 @@ export default class CandidateArchiveFileHasher extends Module { ); // {@link CandidateGenerator} would have copied undefined values from the input // file, so we need to modify the expected output file as well for testing - const hashedOutputFile = romWithFiles.getOutputFile().withProps({ + const hashedOutputFile = outputFile.withProps({ size: hashedInputFile.getSize(), crc32: hashedInputFile.getCrc32(), md5: hashedInputFile.getMd5(), diff --git a/src/modules/candidateGenerator.ts b/src/modules/candidateGenerator.ts index 3f3d046d9..baa8ee4ab 100644 --- a/src/modules/candidateGenerator.ts +++ b/src/modules/candidateGenerator.ts @@ -233,21 +233,6 @@ export default class CandidateGenerator extends Module { .filter(([, romWithFiles]) => !romWithFiles) .map(([rom]) => rom); - // If there is a CHD with every .bin file, then assume its .cue file is accurate - if (missingRoms.length > 0 && CandidateGenerator.onlyCueFilesMissingFromChd( - game, - foundRomsWithFiles.map((romWithFiles) => romWithFiles.getRom()), - )) { - const inputChds = foundRomsWithFiles - .map((romWithFiles) => romWithFiles.getOutputFile()) - .filter((file) => file.getFilePath().toLowerCase().endsWith('.chd')) - .filter(ArrayPoly.filterUniqueMapped((file) => file.getFilePath())); - if (inputChds.length === 1) { - this.progressBar.logTrace(`${dat.getNameShort()}: ${game.getName()}: `); - return new ReleaseCandidate(game, release, foundRomsWithFiles); - } - } - // Ignore the Game if not every File is present if (missingRoms.length > 0 && !this.options.getAllowIncompleteSets()) { if (foundRomsWithFiles.length > 0) { @@ -317,13 +302,13 @@ export default class CandidateGenerator extends Module { // Filter to the Archives that contain every ROM in this Game const archivesWithEveryRom = [...inputArchivesToRoms.entries()] - .filter(([archive, roms]) => { + .filter(([inputArchive, roms]) => { if (roms.map((rom) => rom.hashCode()).join(',') === gameRoms.map((rom) => rom.hashCode()).join(',')) { return true; } // If there is a CHD with every .bin file, and we're raw-copying it, then assume its .cue // file is accurate - return archive instanceof Chd + return inputArchive instanceof Chd && !gameRoms.some((rom) => this.options.shouldZipRom(rom)) && !gameRoms.some((rom) => this.options.shouldExtractRom(rom)) && CandidateGenerator.onlyCueFilesMissingFromChd(game, roms); diff --git a/src/modules/datGameInferrer.ts b/src/modules/datGameInferrer.ts index 1921e5f5f..f65d7d246 100644 --- a/src/modules/datGameInferrer.ts +++ b/src/modules/datGameInferrer.ts @@ -190,12 +190,12 @@ export default class DATGameInferrer extends Module { .map((binFile) => path.join(path.dirname(cueFile.getFilePath()), binFile.name)) .map((binFilePath) => rawFilePathsToFiles.get(binFilePath)) .filter(ArrayPoly.filterNotNullish); - - if (binFiles.length > 0) { - const gameName = DATGameInferrer.getGameName(cueFile); - return [gameName, [cueFile, ...binFiles]]; + if (binFiles.length === 0) { + return undefined; } - return undefined; + + const gameName = DATGameInferrer.getGameName(cueFile); + return [gameName, [cueFile, ...binFiles]]; } catch { return undefined; } @@ -239,12 +239,12 @@ export default class DATGameInferrer extends Module { .map((trackFilePath) => path.join(path.dirname(gdiFile.getFilePath()), trackFilePath)) .map((trackFilePath) => rawFilePathsToFiles.get(trackFilePath)) .filter(ArrayPoly.filterNotNullish); - - if (trackFiles.length > 0) { - const gameName = DATGameInferrer.getGameName(gdiFile); - return [gameName, [gdiFile, ...trackFiles]]; + if (trackFiles.length === 0) { + return undefined; } - return undefined; + + const gameName = DATGameInferrer.getGameName(gdiFile); + return [gameName, [gdiFile, ...trackFiles]]; } catch { return undefined; } diff --git a/src/modules/fixdatCreator.ts b/src/modules/fixdatCreator.ts index 0ec411e15..240d055f2 100644 --- a/src/modules/fixdatCreator.ts +++ b/src/modules/fixdatCreator.ts @@ -60,7 +60,7 @@ export default class FixdatCreator extends Module { try { fixdatDir = this.getDatOutputDirRoot(originalDat); } catch (error) { - this.progressBar.logWarn(`failed to: ${error}`); + this.progressBar.logWarn(`${originalDat.getNameShort()}: failed to get output directory: ${error}`); } } if (!await fsPoly.exists(fixdatDir)) { diff --git a/test/igir.test.ts b/test/igir.test.ts index 52879adbb..7c3c7b8c5 100644 --- a/test/igir.test.ts +++ b/test/igir.test.ts @@ -10,7 +10,12 @@ import ArrayPoly from '../src/polyfill/arrayPoly.js'; import fsPoly from '../src/polyfill/fsPoly.js'; import { ChecksumBitmask } from '../src/types/files/fileChecksums.js'; import FileFactory from '../src/types/files/fileFactory.js'; -import Options, { FixExtension, GameSubdirMode, OptionsProps } from '../src/types/options.js'; +import Options, { + FixExtension, + GameSubdirMode, + InputChecksumArchivesMode, + OptionsProps, +} from '../src/types/options.js'; import ProgressBarFake from './console/progressBarFake.js'; interface TestOutput { @@ -143,17 +148,20 @@ describe('with explicit DATs', () => { }), new Logger(LogLevel.NEVER)).main()).rejects.toThrow(/no valid dat files/i); }); - it('should copy and test', async () => { + it('should copy and test, without caching', async () => { await copyFixturesToTemp(async (inputTemp, outputTemp) => { const result = await runIgir({ commands: ['copy', 'test'], dat: [path.join(inputTemp, 'dats', '*')], input: [path.join(inputTemp, 'roms')], inputExclude: [path.join(inputTemp, 'roms', 'discs')], // test archive scanning + matching + inputChecksumArchives: InputChecksumArchivesMode[InputChecksumArchivesMode.NEVER] + .toLowerCase(), output: outputTemp, dirDatName: true, dirGameSubdir: GameSubdirMode[GameSubdirMode.MULTIPLE].toLowerCase(), fixExtension: FixExtension[FixExtension.AUTO].toLowerCase(), + disableCache: true, }); expect(result.outputFilesAndCrcs).toEqual([ @@ -178,7 +186,6 @@ describe('with explicit DATs', () => { [`${path.join('One', 'GD-ROM.chd')}|track02.raw`, 'abc178d5'], [`${path.join('One', 'GD-ROM.chd')}|track03.bin`, '61a363f1'], [`${path.join('One', 'GD-ROM.chd')}|track04.bin`, 'fc5ff5a0'], - [`${path.join('One', 'Lorem Ipsum.zip')}|loremipsum.rom`, '70856527'], [path.join('One', 'One Three', 'One.rom'), 'f817a89f'], [path.join('One', 'One Three', 'Three.rom'), 'ff46c5d8'], [`${path.join('One', 'Three Four Five', '2048')}|`, 'xxxxxxxx'], // hard disk @@ -209,16 +216,19 @@ describe('with explicit DATs', () => { }); }); - it('should copy a 1G1R set', async () => { + it('should copy a 1G1R set, with a custom cache path', async () => { await copyFixturesToTemp(async (inputTemp, outputTemp) => { const result = await runIgir({ commands: ['copy'], dat: [path.join(inputTemp, 'dats', 'one.dat')], input: [path.join(inputTemp, 'roms')], + inputChecksumArchives: InputChecksumArchivesMode[InputChecksumArchivesMode.ALWAYS] + .toLowerCase(), output: outputTemp, dirGameSubdir: GameSubdirMode[GameSubdirMode.MULTIPLE].toLowerCase(), single: true, preferParent: true, + cachePath: inputTemp, }); expect(result.outputFilesAndCrcs).toEqual([ @@ -784,7 +794,7 @@ describe('with explicit DATs', () => { [`${path.join('One', 'CD-ROM.chd')}|CD-ROM.cue -> ${path.join('', 'chd', 'CD-ROM.chd')}|CD-ROM.cue`, 'xxxxxxxx'], [`${path.join('One', 'Fizzbuzz.nes')} -> ${path.join('', 'raw', 'fizzbuzz.nes')}`, '370517b5'], [`${path.join('One', 'Foobar.lnx')} -> ${path.join('', 'foobar.lnx')}`, 'b22c9747'], - [`${path.join('One', 'GameCube NKit ISO.nkit.iso')}|5bc2ce5b.iso -> ${path.join('', 'nkit', '5bc2ce5b.nkit.iso')}|5bc2ce5b.iso`, '5bc2ce5b'], + [`${path.join('One', 'GameCube NKit ISO.nkit.iso')}|GameCube NKit ISO.iso -> ${path.join('', 'nkit', '5bc2ce5b.nkit.iso')}|GameCube NKit ISO.iso`, '5bc2ce5b'], [`${path.join('One', 'GD-ROM.chd')}|GD-ROM.gdi -> ${path.join('', 'chd', 'GD-ROM.chd')}|GD-ROM.gdi`, 'f16f621c'], [`${path.join('One', 'GD-ROM.chd')}|track01.bin -> ${path.join('', 'chd', 'GD-ROM.chd')}|track01.bin`, '9796ed9a'], [`${path.join('One', 'GD-ROM.chd')}|track02.raw -> ${path.join('', 'chd', 'GD-ROM.chd')}|track02.raw`, 'abc178d5'], @@ -1053,13 +1063,13 @@ 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 inputDir = path.join(inputTemp, 'roms'); const inputBefore = await walkWithCrc(inputDir, inputDir); await runIgir({ commands: ['move', 'test'], input: [inputDir], - output: inputDir, + output: '{inputDirname}', }); await expect(walkWithCrc(inputDir, inputDir)).resolves.toEqual(inputBefore); diff --git a/test/polyfill/fsPoly.test.ts b/test/polyfill/fsPoly.test.ts index fc7eab67c..56b28c6a2 100644 --- a/test/polyfill/fsPoly.test.ts +++ b/test/polyfill/fsPoly.test.ts @@ -29,6 +29,52 @@ describe('isDirectory', () => { }); }); +describe('isHardlink', () => { + it('should return true for a hardlink', async () => { + const tempFileTarget = await fsPoly.mktemp(path.join(Temp.getTempDir(), 'target')); + const tempFileLink = await fsPoly.mktemp(path.join(Temp.getTempDir(), 'link')); + + try { + await fsPoly.touch(tempFileTarget); + + await fsPoly.hardlink(tempFileTarget, tempFileLink); + await expect(fsPoly.isHardlink(tempFileLink)).resolves.toEqual(true); + await expect(fsPoly.isHardlink(tempFileTarget)).resolves.toEqual(true); + } finally { + await fsPoly.rm(tempFileTarget, { force: true }); + await fsPoly.rm(tempFileLink, { force: true }); + } + }); + + it('should return false for a symlink', async () => { + const tempFileTarget = await fsPoly.mktemp(path.join(Temp.getTempDir(), 'target')); + const tempFileLink = await fsPoly.mktemp(path.join(Temp.getTempDir(), 'link')); + + try { + await fsPoly.touch(tempFileTarget); + + await fsPoly.symlink(tempFileTarget, tempFileLink); + await expect(fsPoly.isHardlink(tempFileLink)).resolves.toEqual(false); + await expect(fsPoly.isHardlink(tempFileTarget)).resolves.toEqual(false); + } finally { + await fsPoly.rm(tempFileTarget, { force: true }); + await fsPoly.rm(tempFileLink, { force: true }); + } + }); + + it('should return false for a file', async () => { + const tempFile = await fsPoly.mktemp(path.join(Temp.getTempDir(), 'temp')); + await fsPoly.touch(tempFile); + await expect(fsPoly.isHardlink(tempFile)).resolves.toEqual(false); + await fsPoly.rm(tempFile); + }); + + it('should return false for non-existent file', async () => { + const tempFile = await fsPoly.mktemp(path.join(Temp.getTempDir(), 'temp')); + await expect(fsPoly.isHardlink(tempFile)).resolves.toEqual(false); + }); +}); + describe('hardlink', () => { it('should create a hardlink', async () => { const tempFileTarget = await fsPoly.mktemp(path.join(Temp.getTempDir(), 'target'));