fix(cli): dry run being inaccurate (#9088)

cli/src/commands/asset.ts

@@ -141,7 +141,7 @@ const uploadFiles = async (files: string[], { dryRun, concurrency }: UploadOptio
 
   if (dryRun) {
     console.log(`Would have uploaded ${files.length} asset${s(files.length)} (${byteSize(totalSize)})`);
-    return [];
+    return files.map((filepath) => ({ id: '', filepath }));
   }
 
   const uploadProgress = new SingleBar(
@@ -244,7 +244,7 @@ const deleteFiles = async (files: string[], options: UploadOptionsDto): Promise<
   }
 
   if (options.dryRun) {
-    console.log(`Would now have deleted assets, but skipped due to dry run`);
+    console.log(`Would have deleted ${files.length} local asset${s(files.length)}`);
     return;
   }
 
@@ -285,7 +285,7 @@ const updateAlbums = async (assets: Asset[], options: UploadOptionsDto) => {
   if (dryRun) {
     // TODO print asset counts for new albums
     console.log(`Would have created ${newAlbums.size} new album${s(newAlbums.size)}`);
-    console.log(`Would have updated ${assets.length} asset${s(assets.length)}`);
+    console.log(`Would have updated albums of ${assets.length} asset${s(assets.length)}`);
     return;
   }
 

cli/src/index.ts

@@ -60,7 +60,8 @@ program
   .addOption(
     new Option('-n, --dry-run', "Don't perform any actions, just show what will be done")
       .env('IMMICH_DRY_RUN')
-      .default(false),
+      .default(false)
+      .conflicts('skipHash'),
   )
   .addOption(
     new Option('-c, --concurrency <number>', 'Number of assets to upload at the same time')

e2e/src/cli/specs/upload.e2e-spec.ts

@@ -51,7 +51,7 @@ describe(`immich upload`, () => {
           expect.stringContaining('All assets were already uploaded, nothing to do'),
         ]),
       );
-      expect(first.exitCode).toBe(0);
+      expect(second.exitCode).toBe(0);
     });
 
     it('should skip files that do not exist', async () => {
@@ -63,6 +63,44 @@ describe(`immich upload`, () => {
       const assets = await getAllAssets({}, { headers: asKeyAuth(key) });
       expect(assets.length).toBe(0);
     });
+
+    it('should have accurate dry run', async () => {
+      const { stderr, stdout, exitCode } = await immichCli([
+        'upload',
+        `${testAssetDir}/albums/nature/silver_fir.jpg`,
+        '--dry-run',
+      ]);
+      expect(stderr).toBe('');
+      expect(stdout.split('\n')).toEqual(
+        expect.arrayContaining([expect.stringContaining('Would have uploaded 1 asset')]),
+      );
+      expect(exitCode).toBe(0);
+
+      const assets = await getAllAssets({}, { headers: asKeyAuth(key) });
+      expect(assets.length).toBe(0);
+    });
+
+    it('dry run should handle duplicates', async () => {
+      const first = await immichCli(['upload', `${testAssetDir}/albums/nature/silver_fir.jpg`]);
+      expect(first.stderr).toBe('');
+      expect(first.stdout.split('\n')).toEqual(
+        expect.arrayContaining([expect.stringContaining('Successfully uploaded 1 new asset')]),
+      );
+      expect(first.exitCode).toBe(0);
+
+      const assets = await getAllAssets({}, { headers: asKeyAuth(key) });
+      expect(assets.length).toBe(1);
+
+      const second = await immichCli(['upload', `${testAssetDir}/albums/nature/`, '--dry-run']);
+      expect(second.stderr).toBe('');
+      expect(second.stdout.split('\n')).toEqual(
+        expect.arrayContaining([
+          expect.stringContaining('Found 8 new files and 1 duplicate'),
+          expect.stringContaining('Would have uploaded 8 assets'),
+        ]),
+      );
+      expect(second.exitCode).toBe(0);
+    });
   });
 
   describe('immich upload --recursive', () => {
@@ -136,6 +174,31 @@ describe(`immich upload`, () => {
       expect(albums2.length).toBe(1);
       expect(albums2[0].albumName).toBe('nature');
     });
+
+    it('should have accurate dry run', async () => {
+      const { stderr, stdout, exitCode } = await immichCli([
+        'upload',
+        `${testAssetDir}/albums/nature/`,
+        '--recursive',
+        '--album',
+        '--dry-run',
+      ]);
+      expect(stdout.split('\n')).toEqual(
+        expect.arrayContaining([
+          expect.stringContaining('Would have uploaded 9 assets'),
+          expect.stringContaining('Would have created 1 new album'),
+          expect.stringContaining('Would have updated albums of 9 assets'),
+        ]),
+      );
+      expect(stderr).toBe('');
+      expect(exitCode).toBe(0);
+
+      const assets = await getAllAssets({}, { headers: asKeyAuth(key) });
+      expect(assets.length).toBe(0);
+
+      const albums = await getAllAlbums({}, { headers: asKeyAuth(key) });
+      expect(albums.length).toBe(0);
+    });
   });
 
   describe('immich upload --recursive --album-name=e2e', () => {
@@ -163,6 +226,31 @@ describe(`immich upload`, () => {
       expect(albums.length).toBe(1);
       expect(albums[0].albumName).toBe('e2e');
     });
+
+    it('should have accurate dry run', async () => {
+      const { stderr, stdout, exitCode } = await immichCli([
+        'upload',
+        `${testAssetDir}/albums/nature/`,
+        '--recursive',
+        '--album-name=e2e',
+        '--dry-run',
+      ]);
+      expect(stdout.split('\n')).toEqual(
+        expect.arrayContaining([
+          expect.stringContaining('Would have uploaded 9 assets'),
+          expect.stringContaining('Would have created 1 new album'),
+          expect.stringContaining('Would have updated albums of 9 assets'),
+        ]),
+      );
+      expect(stderr).toBe('');
+      expect(exitCode).toBe(0);
+
+      const assets = await getAllAssets({}, { headers: asKeyAuth(key) });
+      expect(assets.length).toBe(0);
+
+      const albums = await getAllAlbums({}, { headers: asKeyAuth(key) });
+      expect(albums.length).toBe(0);
+    });
   });
 
   describe('immich upload --delete', () => {
@@ -191,6 +279,32 @@ describe(`immich upload`, () => {
       const assets = await getAllAssets({}, { headers: asKeyAuth(key) });
       expect(assets.length).toBe(9);
     });
+
+    it('should have accurate dry run', async () => {
+      await mkdir(`/tmp/albums/nature`, { recursive: true });
+      const filesToLink = await readdir(`${testAssetDir}/albums/nature`);
+      for (const file of filesToLink) {
+        await symlink(`${testAssetDir}/albums/nature/${file}`, `/tmp/albums/nature/${file}`);
+      }
+
+      const { stderr, stdout, exitCode } = await immichCli(['upload', `/tmp/albums/nature`, '--delete', '--dry-run']);
+
+      const files = await readdir(`/tmp/albums/nature`);
+      await rm(`/tmp/albums/nature`, { recursive: true });
+      expect(files.length).toBe(9);
+
+      expect(stdout.split('\n')).toEqual(
+        expect.arrayContaining([
+          expect.stringContaining('Would have uploaded 9 assets'),
+          expect.stringContaining('Would have deleted 9 local assets'),
+        ]),
+      );
+      expect(stderr).toBe('');
+      expect(exitCode).toBe(0);
+
+      const assets = await getAllAssets({}, { headers: asKeyAuth(key) });
+      expect(assets.length).toBe(0);
+    });
   });
 
   describe('immich upload --skip-hash', () => {
@@ -217,6 +331,22 @@ describe(`immich upload`, () => {
       const assets = await getAllAssets({}, { headers: asKeyAuth(key) });
       expect(assets.length).toBe(1);
     });
+
+    it('should throw an error if attempting dry run', async () => {
+      const { stderr, stdout, exitCode } = await immichCli([
+        'upload',
+        `${testAssetDir}/albums/nature/`,
+        '--skip-hash',
+        '--dry-run',
+      ]);
+
+      expect(stdout).toBe('');
+      expect(stderr).toEqual(`error: option '-n, --dry-run' cannot be used with option '-h, --skip-hash'`);
+      expect(exitCode).not.toBe(0);
+
+      const assets = await getAllAssets({}, { headers: asKeyAuth(key) });
+      expect(assets.length).toBe(0);
+    });
   });
 
   describe('immich upload --concurrency <number>', () => {
@@ -302,5 +432,27 @@ describe(`immich upload`, () => {
       const assets = await getAllAssets({}, { headers: asKeyAuth(key) });
       expect(assets.length).toBe(1);
     });
+
+    it('should have accurate dry run', async () => {
+      const { stderr, stdout, exitCode } = await immichCli([
+        'upload',
+        `${testAssetDir}/albums/nature/`,
+        '--ignore',
+        'silver_fir.jpg',
+        '--dry-run',
+      ]);
+
+      expect(stderr).toBe('');
+      expect(stdout.split('\n')).toEqual(
+        expect.arrayContaining([
+          'Found 8 new files and 0 duplicates',
+          expect.stringContaining('Would have uploaded 8 assets'),
+        ]),
+      );
+      expect(exitCode).toBe(0);
+
+      const assets = await getAllAssets({}, { headers: asKeyAuth(key) });
+      expect(assets.length).toBe(0);
+    });
   });
 });