Fix file.get_diff for remote files

Once the files were cached, the function was still trying to open the
paths passed into it, not the cached paths. This fixes that and adds
additional unit tests.

salt/modules/file.py

@@ -4988,7 +4988,7 @@ def get_diff(file1,
         )
 
     args = []
-    for filename in files:
+    for filename in paths:
         try:
             with salt.utils.files.fopen(filename, 'rb') as fp_:
                 args.append(fp_.readlines())
@@ -5006,12 +5006,12 @@ def get_diff(file1,
         elif not show_changes:
             ret = '<show_changes=False>'
         else:
-            bdiff = _binary_replace(*files)
+            bdiff = _binary_replace(*paths)
             if bdiff:
                 ret = bdiff
             else:
                 if show_filenames:
-                    args.extend(files)
+                    args.extend(paths)
                 ret = __utils__['stringutils.get_diff'](*args)
         return ret
     return ''

tests/unit/modules/test_file.py

@@ -910,6 +910,17 @@ class FileModuleTestCase(TestCase, LoaderModuleMockMixin):
             baz
             яйца
             ''')
+        diff_result = textwrap.dedent('''\
+            --- text1
+            +++ text2
+            @@ -1,4 +1,4 @@
+             foo
+             bar
+             baz
+            -спам
+            +яйца
+            ''')
+
         # The below two variables are 8 bytes of data pulled from /dev/urandom
         binary1 = b'\xd4\xb2\xa6W\xc6\x8e\xf5\x0f'
         binary2 = b',\x13\x04\xa5\xb0\x12\xdf%'
@@ -940,7 +951,7 @@ class FileModuleTestCase(TestCase, LoaderModuleMockMixin):
         # pylint: enable=no-self-argument
 
         fopen = MagicMock(side_effect=lambda x, *args, **kwargs: MockFopen(x))
-        cache_file = MagicMock(side_effect=lambda x, *args, **kwargs: x)
+        cache_file = MagicMock(side_effect=lambda x, *args, **kwargs: x.split('/')[-1])
 
         # Mocks for __utils__['files.is_text']
         mock_text_text = MagicMock(side_effect=[True, True])
@@ -960,19 +971,18 @@ class FileModuleTestCase(TestCase, LoaderModuleMockMixin):
 
                 # Non-identical files
                 ret = filemod.get_diff('text1', 'text2')
-                self.assertEqual(
-                    ret,
-                    textwrap.dedent('''\
-                        --- text1
-                        +++ text2
-                        @@ -1,4 +1,4 @@
-                         foo
-                         bar
-                         baz
-                        -спам
-                        +яйца
-                        ''')
-                )
+                self.assertEqual(ret, diff_result)
+
+                # Repeat the above test with remote file paths. The expectation
+                # is that the cp.cache_file mock will ensure that we are not
+                # trying to do an fopen on the salt:// URL, but rather the
+                # "cached" file path we've mocked.
+                with patch.object(filemod, '_binary_replace',
+                                  MagicMock(return_value='')):
+                    ret = filemod.get_diff('salt://text1', 'salt://text1')
+                    self.assertEqual(ret, '')
+                    ret = filemod.get_diff('salt://text1', 'salt://text2')
+                    self.assertEqual(ret, diff_result)
 
             # Test diffing two binary files
             with patch.dict(filemod.__utils__, {'files.is_text': mock_bin_bin}):