danger / danger/danger-js

danger.git.modified_files contains undefined

Open
#304 1 comment 0 reactions 0 assignees View on GitHub
bug
Dominant language
TypeScript
Stars
5.5k
Forks
381
Avg merge
2h 8m
Merged PRs (30d)
6

Description

It looks like `danger.git.modified_files` (and potentially `deleted_files` or `created_files`?) can include `undefined` as a filename.

The bug was discovered in a private repo at work, so I can't share links / haven't been able to dig deeply into the cause yet, but I did log out the DSL [right before `danger.git` is created](https://github.com/danger/danger-js/blob/a6223c12b6a84e9308727745768d4c89baad3573/source/platforms/github/GitHubGit.ts#L169):

```
{ modifiedDiffs:
[ { chunks: [Object],
deletions: 1,
additions: 1,
from: 'dangerfile.ts',
to: 'dangerfile.ts',
index: [Object] },
{ chunks: [Object],
deletions: 8,
additions: 32,
from: 'sql/[redacted].sql',
to: 'sql/[redacted].sql',
index: [Object] },
{ chunks: [],
deletions: 1,
additions: 5,
from: '[redacted content from sql/[redacted].sql]' } ],
addedDiffs: [],
removedDiffs: [] }
```

There `danger.git.modified_files` equals:

```
[ 'dangerfile.ts', 'sql/[redacted].sql', undefined ]
```

Would it make sense to filter out any falsy values before returning `danger.git.(modified|created|deleted)_files`? It seems like a band aid solution, but it also was unexpected to try to execute the following code and seeing Danger fail:

```ts
const testFiles = danger.git.modified_files
.concat(danger.git.created_files)
// `path` may be undefined here
.filter(path => path.endsWith('.test.ts'))
```

Would love suggestions on preventing `undefined` from sneaking through. Will try to get redacted GitHub API data from my work repo and push up a failing test too. 🙂

Contributor guide

Open the contributing guide

Research direction

Start at source/platforms/github/GitHubGit.ts around the point where the DSL is logged before danger.git is created. Inspect how modifiedDiffs, addedDiffs, and removedDiffs become the modified_files, created_files, and deleted_files arrays, then reproduce the reported undefined filename with representative diff data. Done means the file lists no longer contain undefined values and a regression test covers the case.

Written by the indexing model from the issue text.

Assessment

Tech stack
github, typescript
Domain
api
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.