From 94d1ce10c40f9395fa77e936d3aadbabc807c32c Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 3 Oct 2023 16:18:48 +0200 Subject: [PATCH] apply review feedback --- src/utils/__tests__/symlink.test.ts | 262 ++++++++++++++-------------- src/utils/symlink.ts | 35 ++-- 2 files changed, 142 insertions(+), 155 deletions(-) diff --git a/src/utils/__tests__/symlink.test.ts b/src/utils/__tests__/symlink.test.ts index 7c1a4789..f3cf11e1 100644 --- a/src/utils/__tests__/symlink.test.ts +++ b/src/utils/__tests__/symlink.test.ts @@ -68,153 +68,101 @@ describe('createSymlinks', () => { }, true) ); - describe('correctly handles already existing symlinks', () => { - it('when updating an old major version', async () => - withTempDir(async tmpDir => { - // fill the directory with some "previous" versions and symlinks - // (we also need to also the concrete version file to avoid broken symlinks) - - fs.writeFileSync(path.join(tmpDir, '1.0.0.json'), 'x'); - createSymlinks(path.join(tmpDir, '1.0.0.json'), '1.0.0'); - - fs.writeFileSync(path.join(tmpDir, '2.0.0.json'), 'x'); - createSymlinks(path.join(tmpDir, '2.0.0.json'), '2.0.0', '1.0.0'); - - // now update 1.x (minor) - fs.writeFileSync(path.join(tmpDir, '1.5.0.json'), 'x'); - createSymlinks(path.join(tmpDir, '1.5.0.json'), '1.5.0', '2.0.0'); - - // now update a version in between 1.x and 1.5.x - fs.writeFileSync(path.join(tmpDir, '1.2.0.json'), 'x'); - createSymlinks(path.join(tmpDir, '1.2.0.json'), '1.2.0', '2.0.0'); - - // now update 1.5.x (patch) - fs.writeFileSync(path.join(tmpDir, '1.5.1.json'), 'x'); - createSymlinks(path.join(tmpDir, '1.5.1.json'), '1.5.1', '2.0.0'); - - const filesInDir = await fsPromises.readdir(tmpDir); - expect(filesInDir.sort()).toStrictEqual( - [ - '1.0.0.json', - '1.2.0.json', - '1.5.0.json', - '1.5.1.json', - '2.0.0.json', - - '1.json', - '2.json', - - '1.0.json', - '1.2.json', - '1.5.json', - '2.0.json', - - 'latest.json', - ].sort() - ); - - const latestLink = await fsPromises.readlink( - path.join(tmpDir, 'latest.json') - ); - const major1Link = await fsPromises.readlink( - path.join(tmpDir, '1.json') - ); - const major2Link = await fsPromises.readlink( - path.join(tmpDir, '2.json') - ); - const minor10Link = await fsPromises.readlink( - path.join(tmpDir, '1.0.json') - ); - const minor12Link = await fsPromises.readlink( - path.join(tmpDir, '1.2.json') - ); - const minor15Link = await fsPromises.readlink( - path.join(tmpDir, '1.5.json') - ); - const minor20Link = await fsPromises.readlink( - path.join(tmpDir, '2.0.json') - ); - - expect(latestLink).toBe('2.0.0.json'); - - expect(major1Link).toBe('1.5.1.json'); - expect(major2Link).toBe('2.0.0.json'); - - expect(minor10Link).toBe('1.0.0.json'); - expect(minor12Link).toBe('1.2.0.json'); - expect(minor15Link).toBe('1.5.1.json'); - expect(minor20Link).toBe('2.0.0.json'); - }, true)); - - it('when updating a previous minor version on the same major', async () => - withTempDir(async tmpDir => { - fs.writeFileSync(path.join(tmpDir, '1.0.0.json'), 'x'); - createSymlinks(path.join(tmpDir, '1.0.0.json'), '1.0.0'); - - fs.writeFileSync(path.join(tmpDir, '1.1.0.json'), 'x'); - createSymlinks(path.join(tmpDir, '1.1.0.json'), '1.1.0', '1.0.0'); - - fs.writeFileSync(path.join(tmpDir, '1.0.1.json'), 'x'); - createSymlinks(path.join(tmpDir, '1.0.1.json'), '1.0.1', '1.1.0'); - - const filesInDir = await fsPromises.readdir(tmpDir); - expect(filesInDir.sort()).toStrictEqual( - [ - '1.0.0.json', - '1.0.1.json', - '1.1.0.json', - - '1.json', - - '1.0.json', - '1.1.json', - - 'latest.json', - ].sort() - ); - - const latestLink = await fsPromises.readlink( - path.join(tmpDir, 'latest.json') - ); - const major1Link = await fsPromises.readlink( - path.join(tmpDir, '1.json') - ); - const minor10Link = await fsPromises.readlink( - path.join(tmpDir, '1.0.json') - ); - const minor11Link = await fsPromises.readlink( - path.join(tmpDir, '1.1.json') - ); - - expect(latestLink).toBe('1.1.0.json'); - expect(major1Link).toBe('1.1.0.json'); - expect(minor10Link).toBe('1.0.1.json'); - expect(minor11Link).toBe('1.1.0.json'); - }, true)); - }); - - // This is quite an edge case but nevertheless good to know it's covered: - it('when updating a previous patch version on the same minor', async () => + it('handles updating an old major version', async () => + withTempDir(async tmpDir => { + // fill the directory with some "previous" versions and symlinks + // (we also need to also the concrete version file to avoid broken symlinks) + + fs.writeFileSync(path.join(tmpDir, '1.0.0.json'), 'x'); + createSymlinks(path.join(tmpDir, '1.0.0.json'), '1.0.0'); + + fs.writeFileSync(path.join(tmpDir, '2.0.0.json'), 'x'); + createSymlinks(path.join(tmpDir, '2.0.0.json'), '2.0.0', '1.0.0'); + + // now update 1.x (minor) + fs.writeFileSync(path.join(tmpDir, '1.5.0.json'), 'x'); + createSymlinks(path.join(tmpDir, '1.5.0.json'), '1.5.0', '2.0.0'); + + // now update a version in between 1.x and 1.5.x + fs.writeFileSync(path.join(tmpDir, '1.2.0.json'), 'x'); + createSymlinks(path.join(tmpDir, '1.2.0.json'), '1.2.0', '2.0.0'); + + // now update 1.5.x (patch) + fs.writeFileSync(path.join(tmpDir, '1.5.1.json'), 'x'); + createSymlinks(path.join(tmpDir, '1.5.1.json'), '1.5.1', '2.0.0'); + + const filesInDir = await fsPromises.readdir(tmpDir); + expect(filesInDir.sort()).toStrictEqual( + [ + '1.0.0.json', + '1.2.0.json', + '1.5.0.json', + '1.5.1.json', + '2.0.0.json', + + '1.json', + '2.json', + + '1.0.json', + '1.2.json', + '1.5.json', + '2.0.json', + + 'latest.json', + ].sort() + ); + + const latestLink = await fsPromises.readlink( + path.join(tmpDir, 'latest.json') + ); + const major1Link = await fsPromises.readlink(path.join(tmpDir, '1.json')); + const major2Link = await fsPromises.readlink(path.join(tmpDir, '2.json')); + const minor10Link = await fsPromises.readlink( + path.join(tmpDir, '1.0.json') + ); + const minor12Link = await fsPromises.readlink( + path.join(tmpDir, '1.2.json') + ); + const minor15Link = await fsPromises.readlink( + path.join(tmpDir, '1.5.json') + ); + const minor20Link = await fsPromises.readlink( + path.join(tmpDir, '2.0.json') + ); + + expect(latestLink).toBe('2.0.0.json'); + + expect(major1Link).toBe('1.5.1.json'); + expect(major2Link).toBe('2.0.0.json'); + + expect(minor10Link).toBe('1.0.0.json'); + expect(minor12Link).toBe('1.2.0.json'); + expect(minor15Link).toBe('1.5.1.json'); + expect(minor20Link).toBe('2.0.0.json'); + }, true)); + + it('handles updating a previous minor version on the same major', async () => withTempDir(async tmpDir => { fs.writeFileSync(path.join(tmpDir, '1.0.0.json'), 'x'); createSymlinks(path.join(tmpDir, '1.0.0.json'), '1.0.0'); - fs.writeFileSync(path.join(tmpDir, '1.0.2.json'), 'x'); - createSymlinks(path.join(tmpDir, '1.0.2.json'), '1.0.2', '1.0.0'); + fs.writeFileSync(path.join(tmpDir, '1.1.0.json'), 'x'); + createSymlinks(path.join(tmpDir, '1.1.0.json'), '1.1.0', '1.0.0'); fs.writeFileSync(path.join(tmpDir, '1.0.1.json'), 'x'); - createSymlinks(path.join(tmpDir, '1.0.1.json'), '1.0.1', '1.0.2'); + createSymlinks(path.join(tmpDir, '1.0.1.json'), '1.0.1', '1.1.0'); const filesInDir = await fsPromises.readdir(tmpDir); expect(filesInDir.sort()).toStrictEqual( [ '1.0.0.json', '1.0.1.json', - '1.0.2.json', + '1.1.0.json', '1.json', '1.0.json', + '1.1.json', 'latest.json', ].sort() @@ -227,9 +175,53 @@ describe('createSymlinks', () => { const minor10Link = await fsPromises.readlink( path.join(tmpDir, '1.0.json') ); + const minor11Link = await fsPromises.readlink( + path.join(tmpDir, '1.1.json') + ); - expect(latestLink).toBe('1.0.2.json'); - expect(major1Link).toBe('1.0.2.json'); - expect(minor10Link).toBe('1.0.2.json'); - })); + expect(latestLink).toBe('1.1.0.json'); + expect(major1Link).toBe('1.1.0.json'); + expect(minor10Link).toBe('1.0.1.json'); + expect(minor11Link).toBe('1.1.0.json'); + }, true)); }); + +// This is quite an edge case but nevertheless good to know it's covered: +it('handles updating a previous patch version on the same minor', async () => + withTempDir(async tmpDir => { + fs.writeFileSync(path.join(tmpDir, '1.0.0.json'), 'x'); + createSymlinks(path.join(tmpDir, '1.0.0.json'), '1.0.0'); + + fs.writeFileSync(path.join(tmpDir, '1.0.2.json'), 'x'); + createSymlinks(path.join(tmpDir, '1.0.2.json'), '1.0.2', '1.0.0'); + + fs.writeFileSync(path.join(tmpDir, '1.0.1.json'), 'x'); + createSymlinks(path.join(tmpDir, '1.0.1.json'), '1.0.1', '1.0.2'); + + const filesInDir = await fsPromises.readdir(tmpDir); + expect(filesInDir.sort()).toStrictEqual( + [ + '1.0.0.json', + '1.0.1.json', + '1.0.2.json', + + '1.json', + + '1.0.json', + + 'latest.json', + ].sort() + ); + + const latestLink = await fsPromises.readlink( + path.join(tmpDir, 'latest.json') + ); + const major1Link = await fsPromises.readlink(path.join(tmpDir, '1.json')); + const minor10Link = await fsPromises.readlink( + path.join(tmpDir, '1.0.json') + ); + + expect(latestLink).toBe('1.0.2.json'); + expect(major1Link).toBe('1.0.2.json'); + expect(minor10Link).toBe('1.0.2.json'); + })); diff --git a/src/utils/symlink.ts b/src/utils/symlink.ts index 5d849a61..d8649778 100644 --- a/src/utils/symlink.ts +++ b/src/utils/symlink.ts @@ -54,7 +54,7 @@ export function createSymlinks( versionGreaterOrEqualThan(parsedNewVersion, parsedOldVersion) ) { logger.debug( - `${parsedOldVersion ? 'Updating' : 'Adding'} symlink for "latest.json" ${ + `Updating symlink "latest.json" ${ parsedOldVersion ? `from ${oldVersion} ` : '' }to "${newVersion}"` ); @@ -62,10 +62,10 @@ export function createSymlinks( } // Read possibly existing symlinks for major and minor versions of the new version - const newVersionMajorSymlinkedVersion = getExistingSymlinkedVersion( + const existingLinkedMajorVersion = getExistingSymlinkedVersion( path.join(packageDir, `${parsedNewVersion.major}.json`) ); - const newVersionMinorSymlinkedVersion = getExistingSymlinkedVersion( + const existingLinkedMinorVersion = getExistingSymlinkedVersion( path.join( packageDir, `${parsedNewVersion.major}.${parsedNewVersion.minor}.json` @@ -75,16 +75,14 @@ export function createSymlinks( // link {major}.json if there's no link yet for that major // or if the new version is newer than the currently linked one if ( - !newVersionMajorSymlinkedVersion || - versionGreaterOrEqualThan(parsedNewVersion, newVersionMajorSymlinkedVersion) + !existingLinkedMajorVersion || + versionGreaterOrEqualThan(parsedNewVersion, existingLinkedMajorVersion) ) { const majorVersionLink = `${parsedNewVersion.major}.json`; logger.debug( - `${ - newVersionMajorSymlinkedVersion ? 'Updating' : 'Adding' - } symlink for "${majorVersionLink}" ${ - newVersionMajorSymlinkedVersion - ? `from version "${semVerToString(newVersionMajorSymlinkedVersion)}" ` + `Updating symlink "${majorVersionLink}" ${ + existingLinkedMajorVersion + ? `from version "${semVerToString(existingLinkedMajorVersion)}" ` : '' }to "${newVersion}"` ); @@ -94,16 +92,14 @@ export function createSymlinks( // link {minor}.json if there's no link yet for that minor // or if the new version is newer than the currently linked one if ( - !newVersionMinorSymlinkedVersion || - versionGreaterOrEqualThan(parsedNewVersion, newVersionMinorSymlinkedVersion) + !existingLinkedMinorVersion || + versionGreaterOrEqualThan(parsedNewVersion, existingLinkedMinorVersion) ) { const minorVersionLink = `${parsedNewVersion.major}.${parsedNewVersion.minor}.json`; logger.debug( - `${ - newVersionMinorSymlinkedVersion ? 'Updating' : 'Adding' - } symlink for "${minorVersionLink}" ${ - newVersionMinorSymlinkedVersion - ? `from version "${semVerToString(newVersionMinorSymlinkedVersion)}" ` + `Updating symlink "${minorVersionLink}" ${ + existingLinkedMinorVersion + ? `from version "${semVerToString(existingLinkedMinorVersion)}" ` : '' }to "${newVersion}"` ); @@ -115,10 +111,9 @@ function getExistingSymlinkedVersion(symlinkPath: string): SemVer | null { try { // using lstat instead of exists because broken symlinks return false for exists fs.lstatSync(symlinkPath); - const linkedFile = fs.readlinkSync(symlinkPath); - return parseVersion(path.basename(linkedFile)); } catch { // this means the symlink doesn't exist } - return null; + const linkedFile = fs.readlinkSync(symlinkPath); + return parseVersion(path.basename(linkedFile)); }