Merge pull request #3763 from mr-cal/test-pack-directory

pack: check if output directory is current working directory

snapcraft/pack.py

@@ -62,12 +62,14 @@ def pack_snap(
         output_path = Path(output)
         output_parent = output_path.parent
         if output_path.is_dir():
+            # do not define a snap name if the output is a directory
             output_dir = str(output_path)
-        elif output_parent and output_parent != Path("."):
+        elif output_parent and output_parent.resolve() != Path(".").resolve():
             output_dir = str(output_parent)
             output_file = output_path.name
         else:
-            output_file = output
+            # do not define a directory if the output parent directory is the cwd
+            output_file = output_path.name
 
     command: List[Union[str, Path]] = ["snap", "pack"]
     if output_file is not None:

tests/unit/test_pack.py

@@ -79,9 +79,10 @@ def test_pack_snap_compression(mocker, new_dir):
     ]
 
 
-def test_pack_snap_output_file(mocker, new_dir):
+def test_pack_snap_output_file_output_directory_cwd(mocker, new_dir):
+    """Test `snap pack` when it outputs to the current working directory."""
     mock_run = mocker.patch("subprocess.run")
-    pack.pack_snap(new_dir, output="/tmp/foo")
+    pack.pack_snap(new_dir, output=f"{new_dir}/test.snap")
     assert mock_run.mock_calls == [
         call(
             ["snap", "pack", "--check-skeleton", new_dir],
@@ -90,15 +91,72 @@ def test_pack_snap_output_file(mocker, new_dir):
             universal_newlines=True,
         ),
         call(
-            ["snap", "pack", "--filename", "foo", new_dir, "/tmp"],
+            ["snap", "pack", "--filename", "test.snap", new_dir],
             capture_output=True,
             check=True,
             universal_newlines=True,
         ),
     ]
 
 
-def test_pack_snap_output_dir(mocker, new_dir):
+def test_pack_snap_output_file_output_directory_existing(mocker, new_dir):
+    """Test `snap pack` when it outputs to an existing directory."""
+    mock_run = mocker.patch("subprocess.run")
+    output_directory = new_dir / "output"
+    output_directory.mkdir()
+    assert output_directory.is_dir()
+
+    pack.pack_snap(new_dir, output=output_directory / "test.snap")
+
+    assert mock_run.mock_calls == [
+        call(
+            ["snap", "pack", "--check-skeleton", new_dir],
+            capture_output=True,
+            check=True,
+            universal_newlines=True,
+        ),
+        call(
+            ["snap", "pack", "--filename", "test.snap", new_dir, str(output_directory)],
+            capture_output=True,
+            check=True,
+            universal_newlines=True,
+        ),
+    ]
+
+
+def test_pack_snap_output_file_output_directory_non_existant(mocker, new_dir):
+    """Test `snap pack` when it outputs to a non-existent directory."""
+    mock_run = mocker.patch("subprocess.run")
+    output_directory = new_dir / "output"
+    assert not output_directory.exists()
+
+    pack.pack_snap(new_dir, output=output_directory / "test.snap")
+
+    assert mock_run.mock_calls == [
+        call(
+            ["snap", "pack", "--check-skeleton", new_dir],
+            capture_output=True,
+            check=True,
+            universal_newlines=True,
+        ),
+        call(
+            [
+                "snap",
+                "pack",
+                "--filename",
+                "test.snap",
+                new_dir,
+                str(new_dir / "output"),
+            ],
+            capture_output=True,
+            check=True,
+            universal_newlines=True,
+        ),
+    ]
+
+
+def test_pack_snap_output_directory_not_specified(mocker, new_dir):
+    """Test `snap pack` executes when no output directory is specified."""
     mock_run = mocker.patch("subprocess.run")
     pack.pack_snap(new_dir, output=str(new_dir))
     assert mock_run.mock_calls == [
@@ -117,6 +175,32 @@ def test_pack_snap_output_dir(mocker, new_dir):
     ]
 
 
+def test_pack_snap_output_file_output_directory_existing_no_file_name(mocker, new_dir):
+    """Test `snap pack` when it outputs to an existing directory but no file
+    name is specified."""
+    mock_run = mocker.patch("subprocess.run")
+    output_directory = new_dir / "output"
+    output_directory.mkdir()
+    assert output_directory.is_dir()
+
+    pack.pack_snap(new_dir, output=output_directory)
+
+    assert mock_run.mock_calls == [
+        call(
+            ["snap", "pack", "--check-skeleton", new_dir],
+            capture_output=True,
+            check=True,
+            universal_newlines=True,
+        ),
+        call(
+            ["snap", "pack", new_dir, str(output_directory)],
+            capture_output=True,
+            check=True,
+            universal_newlines=True,
+        ),
+    ]
+
+
 def test_pack_snap_error(mocker, new_dir):
     mocker.patch("subprocess.run", side_effect=subprocess.CalledProcessError(42, "cmd"))
     with pytest.raises(errors.SnapcraftError) as raised: