Renamed files appear within modified_files, but fail when passed to danger.git.diffForFile

[neil-morrison44]

I've got a file within my repo which has had a parent folder renamed (e.g. from /tests/seach/index.spec.js to /tests/search/index.spec.js)

I'm running some tests on the diff of modified files, and this renamed (but not modified) file is turning up within danger.git.modified_files.

It appears that when I run danger.git.diffForFile on the file's path, danger fires off 1 request for the branch file (which succeeds) and 1 request for the master file using the new path (which fails, 404).

I'd expect Danger to either:

• Know that the file has been renamed but not modified, not include it within danger.git.modified_files
• If that's not possible then include it within danger.git.created_files
• Handle renames, request the old version using the old name (if possible?)
• More graceful handling / error reporting of 404s during the requests for diffs

[orta]

I agree, that logic should be pretty feasible to look up using the existing calls we have (e.g. inside the diff we get) then to use that instead of simply looking for the same named file twice

[lawrence-berry]

I've also ran into the issues described above with renamed folders, and agree that it shouldn't be included in modified_files and including it in created_files would make more sense if it's not possible to recognise these files as having been moved rather than modified.

[heatherbooker]

On a related note, when I have a new file added and try to run danger local, diffForFile fails as it tries to run 'git show master:"<my_new_file.ext>"'. It says:

Path '<my_new_file.ext>' exists on disk, but not in 'master'.

But if I change it to JSONPatchForFile, it reports the diff with no errors. So I think it might extend to created_files as well as renamed ones.

[heatherbooker]

I spoke too soon - it reports an empty diff if I use JSONPatchForFile, but at least it doesn't crash. ><

[mAAdhaTTah]

Experiencing this, although I'm not sure it's a major issue. How can we tell that a file has been renamed?

[airtonix]

This hasn't been fixed yet, although #1197 seems to think it has.

[airtonix]

for now we're having to preempt failing cases :(

const invalidFiles = (
    await Promise.all(
      changedPackageJsons.map(async (filename) => {
        try {
          await execasync(`git show master:"${filename}"`);
        } catch (error) {
          return {};
        }
        const diff = await danger.git.JSONDiffForFile(filename);
        return {
          filename,
          versionChanged: Object.keys(diff).includes('version'),
        };
      })
    )
  ).filter(({ versionChanged }) => versionChanged);

[AlexisDurlet]

I see that the ruby version of danger has a dedicated renamed_files getter in its git API. In the js API I find no way to detect a rename and to get the path diff.

What's surprising is that the deleted_files getter prefixes its path with 'a/' --dst-prefix='b/', so I expected to see the same pattern in the renamed files with both path. This alone would already allow to detect a rename and extract the after/before path.

Instead, the renaming is treated like a simple modification and only gives the renamed path.

This is a very confusing behavior.