Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inline not working #176

Closed
steven-diaz opened this issue Jan 17, 2019 · 16 comments
Closed

Inline not working #176

steven-diaz opened this issue Jan 17, 2019 · 16 comments

Comments

@steven-diaz
Copy link

Hi! I just started integrating today with TravisCI and everything seems to be working except for the inline functionality. Version. 1.1.0.

I have two warnings in my PR and they both show up as a single comment block on the PR rather than inline.

Here's some debug info:

Dangerfile.swift

// Dangerfile.swift

import Danger
let danger = Danger()

SwiftLint.lint(inline: true)

travis.yml, relevant portion

script:
  - brew install danger/tap/danger-swift
  - danger-swift ci

travis output, relevant portion

brew install danger/tap/danger-swift
==> Tapping danger/tap
Cloning into '/usr/local/Homebrew/Library/Taps/danger/homebrew-tap'...
Tapped 2 formulae (33 files, 27KB).
==> Installing danger-swift from danger/tap
==> Installing dependencies for danger/tap/danger-swift: danger/tap/danger-js
==> Installing danger/tap/danger-swift dependency: danger/tap/danger-js
==> Downloading https://github.com/danger/danger-js/releases/download/6.1.13/danger-macos.zip
==> Downloading from https://github-production-release-asset-2e65be.s3.amazonaws.com/66146807/bba00100-08e1-11e9-821a-3a349f659128?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAIWNJYAX4CSVEH53A%2F20190117%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20190117T230137Z&X-Amz-Expires=300&X-Amz-Signature=c278cdbc61e177ed45e069c50e214ee16acbae2415b01e59d8bae3bdb5319d37&X-Amz-SignedHeaders=host&actor_id=0&response-content-disposition=attachment%3B%20filename%3Ddanger-macos.zip&response-content-type=application%2Foctet-stream
🍺  /usr/local/Cellar/danger-js/macos: 3 files, 79.9MB, built in 6 seconds
==> Installing danger/tap/danger-swift
==> Downloading https://github.com/danger/danger-swift/archive/1.1.0.tar.gz
==> Downloading from https://codeload.github.com/danger/swift/tar.gz/1.1.0
Warning: Cannot verify integrity of 941b66710bc97d4a61a81d1731cff50b886a61a1eae0f2fd42e1c491c58313e3--swift-1.1.0.tar.gz
A checksum was not provided for this resource
For your reference the SHA256 is: 9f67975999505ae35e9ff3fbe0130f73632366a4f2be415df4c05ada609cb9a4
==> make install PREFIX=/usr/local/Cellar/danger-swift/1.1.0
🍺  /usr/local/Cellar/danger-swift/1.1.0: 17 files, 12.4MB, built in 1 minute 41 seconds
The command "brew install danger/tap/danger-swift" exited with 0.
12.44s$ danger-swift ci
Executing swiftlint lint --quiet --path "Dangerfile.swift" --reporter json
Executing swiftlint lint --quiet --path "Shared/Constants/Constants.swift" --reporter json
Executing pwd 
Found only warnings, not failing the build.
@f-meloni
Copy link
Member

Are the lines parts of your diff?
Swiftlint lints all the files the were modified, but if i'm not wrong you get an inline message just if the line with the warning is one that you actually modified and is part of the diff you see in Github

@orta
Copy link
Member

orta commented Jan 18, 2019

You're not wrong, the GitHub API only allows commenting inline in lines inside the diff

@steven-diaz
Copy link
Author

steven-diaz commented Jan 18, 2019

There is a file in the branch diff with a couple of purposeful violating lines I threw in to test. I just added a new commit with another violation and it just updated the main comment block on the PR with the new warning.

Edit: Added screenshots.

screen shot 2019-01-18 at 10 39 29 am

screen shot 2019-01-18 at 10 39 55 am

@f-meloni
Copy link
Member

Can you please use DEBUG=* danger-swift ci, it should show you on the logs the JSON that is posted back to danger-js, and we can check if it has the file name and the line?

@steven-diaz
Copy link
Author

steven-diaz commented Jan 18, 2019

Sure! Here's the output, I scrubbed the repo name and token, just in case lol:

$ DEBUG=* danger-swift ci
2019-01-18T15:57:47.514Z danger:process_runner Debug mode on for Danger v6.1.13
2019-01-18T15:57:47.516Z danger:process_runner Starting sub-process run
2019-01-18T15:57:47.519Z danger:GitHub::Checks Not using the checks API for GitHub
2019-01-18T15:57:47.519Z danger:GitHubAPI Sending:  https://api.github.com/repos/iphone-app/pulls/6730 { 'Content-Type': 'application/json',
  Accept: 'application/vnd.github.v3.diff',
  Authorization: 'token ' }
2019-01-18T15:57:47.777Z danger:GitHubAPI getPullRequestCommits:: Sending pull request commit request for first page
2019-01-18T15:57:47.778Z danger:GitHubAPI getPullRequestCommits:: Request url generated "repos//iphone-app/pulls/6730/commits"
2019-01-18T15:57:47.778Z danger:GitHubAPI Sending:  https://api.github.com/repos//iphone-app/pulls/6730/commits { 'Content-Type': 'application/json',
  Authorization: 'token ' }
2019-01-18T15:57:48.002Z danger:GitHubAPI getNextPageFromLinkHeader:: Given response does not contain link header for pagination
2019-01-18T15:57:48.003Z danger:GitHubAPI Sending:  https://api.github.com/repos//iphone-app/pulls/6730 { 'Content-Type': 'application/json',
  Authorization: 'token ' }
2019-01-18T15:57:48.176Z danger:GitHubAPI Sending:  https://api.github.com/repos//iphone-app/issues/6730 { 'Content-Type': 'application/json',
  Authorization: 'token ' }
2019-01-18T15:57:48.296Z danger:GitHubAPI getPullRequestCommits:: Sending pull request commit request for first page
2019-01-18T15:57:48.296Z danger:GitHubAPI getPullRequestCommits:: Request url generated "repos//iphone-app/pulls/6730/commits"
2019-01-18T15:57:48.296Z danger:GitHubAPI Sending:  https://api.github.com/repos//iphone-app/pulls/6730/commits { 'Content-Type': 'application/json',
  Authorization: 'token ' }
2019-01-18T15:57:48.482Z danger:GitHubAPI getNextPageFromLinkHeader:: Given response does not contain link header for pagination
2019-01-18T15:57:48.482Z danger:GitHubAPI Sending:  https://api.github.com/repos//iphone-app/pulls/6730/reviews { 'Content-Type': 'application/json',
  Accept: 'application/vnd.github.v3+json',
  Authorization: 'token ' }
2019-01-18T15:57:48.617Z danger:GitHubAPI Sending:  https://api.github.com/repos//iphone-app/pulls/6730/requested_reviewers { 'Content-Type': 'application/json',
  Accept: 'application/vnd.github.v3+json',
  Authorization: 'token ' }
2019-01-18T15:57:48.715Z danger:process_runner Preparing to run: danger-swift
2019-01-18T15:57:48.715Z danger:runDangerSubprocess Running sub-process: danger-swift - 
2019-01-18T15:57:48.723Z danger:runDangerSubprocess Started passing in STDIN via the URL: danger://dsl//var/folders/nz/vv4_9tw56nv9k3tkvyszvwg80000gn/T/danger-dsl.json
2019-01-18T15:57:48.724Z danger:runDangerSubprocess Passed DSL in via STDIN
Ran with: /var/folders/nz/vv4_9tw56nv9k3tkvyszvwg80000gn/T/_tmp_dangerfile.swift /var/folders/nz/vv4_9tw56nv9k3tkvyszvwg80000gn/T/danger-dsl.json /var/folders/nz/vv4_9tw56nv9k3tkvyszvwg80000gn/T/danger-response.json
Decoding the DSL into Swift types
Setting up to dump results
Executing swiftlint lint --quiet --path "Dangerfile.swift" --reporter json
Executing swiftlint lint --quiet --path "Shared/Constants/Constants.swift" --reporter json
Executing pwd 
Sending results back to Danger
2019-01-18T15:58:01.142Z danger:runDangerSubprocess Got JSON URL from STDOUT, results are at: 
danger-results://var/folders/nz/vv4_9tw56nv9k3tkvyszvwg80000gn/T/danger-response.json
Got URL for JSON: /var/folders/nz/vv4_9tw56nv9k3tkvyszvwg80000gn/T/danger-dsl.json
Created a temporary file for the Dangerfile DSL at: /var/folders/nz/vv4_9tw56nv9k3tkvyszvwg80000gn/T/danger-dsl.json
Running Dangerfile at: Dangerfile.swift
Preparing to compile
Running: /usr/bin/swiftc --driver-mode=swift -L /usr/local/lib/danger -I /usr/local/lib/danger -lDanger -L /usr/local/lib/danger -I /usr/local/lib/danger /var/folders/nz/vv4_9tw56nv9k3tkvyszvwg80000gn/T/_tmp_dangerfile.swift /var/folders/nz/vv4_9tw56nv9k3tkvyszvwg80000gn/T/danger-dsl.json /var/folders/nz/vv4_9tw56nv9k3tkvyszvwg80000gn/T/danger-response.json
Completed evaluation
Saving and storing the results at /var/folders/nz/vv4_9tw56nv9k3tkvyszvwg80000gn/T/danger-response.json
2019-01-18T15:58:01.145Z danger:runDangerSubprocess child process exited with code 0
2019-01-18T15:58:01.145Z danger:jsonToDSL Creating pr DSL from JSON
2019-01-18T15:58:01.171Z danger:GitHubGit Got no GH API, had to make it
2019-01-18T15:58:01.172Z danger:GitHubGit Setting up git DSL with:  { repo: '/iphone-app',
  baseSHA: 'fb4f325e441b8a6d91c74ca4ffcf51c5aa370c3e',
  headSHA: '779cfd263ef1f9a67232fe90ce052e99801d29ae',
  getFileContents: [Function],
  getFullDiff: [Function] }
2019-01-18T15:58:01.172Z danger:executor Got results back: { messages: [],
  markdowns: [],
  meta:
   { runtimeHref: 'https://danger.systems',
     runtimeName: 'Danger Swift' },
  fails: [],
  warnings:
   [ { message:
        'Colons should be next to the identifier when specifying a type and next to the key in dictionary literals. (`colon`)',
       file:
        'Users/travis/build//iphone-app/Shared/Constants/Constants.swift',
       line: 19 },
     { message:
        'Colons should be next to the identifier when specifying a type and next to the key in dictionary literals. (`colon`)',
       file:
        'Users/travis/build//iphone-app/Shared/Constants/Constants.swift',
       line: 21 },
     { message:
        'Limit vertical whitespace to a single empty line. Currently 5. (`vertical_whitespace`)',
       file:
        'Users/travis/build//iphone-app/Shared/Constants/Constants.swift',
       line: 18 } ] }
2019-01-18T15:58:01.173Z danger:executor Evaluator settings { stdoutOnly: undefined,
  verbose: undefined,
  jsonOnly: false,
  dangerID: 'default',
  passURLForDSL: true }
2019-01-18T15:58:01.173Z danger:executor Posting to platform: { messages: [],
  markdowns: [],
  meta:
   { runtimeHref: 'https://danger.systems',
     runtimeName: 'Danger Swift' },
  fails: [],
  warnings:
   [ { message:
        'Colons should be next to the identifier when specifying a type and next to the key in dictionary literals. (`colon`)',
       file:
        'Users/travis/build//iphone-app/Shared/Constants/Constants.swift',
       line: 19 },
     { message:
        'Colons should be next to the identifier when specifying a type and next to the key in dictionary literals. (`colon`)',
       file:
        'Users/travis/build//iphone-app/Shared/Constants/Constants.swift',
       line: 21 },
     { message:
        'Limit vertical whitespace to a single empty line. Currently 5. (`vertical_whitespace`)',
       file:
        'Users/travis/build//iphone-app/Shared/Constants/Constants.swift',
       line: 18 } ] }
Found only warnings, not failing the build.
2019-01-18T15:58:01.173Z danger:GitHubAPI Sending:  https://api.github.com/user { 'Content-Type': 'application/json',
  Authorization: 'token ' }
2019-01-18T15:58:01.269Z danger:GitHubAPI getPullRequestCommits:: Sending pull request commit request for first page
2019-01-18T15:58:01.269Z danger:GitHubAPI getPullRequestCommits:: Request url generated "repos//iphone-app/pulls/6730/comments"
2019-01-18T15:58:01.269Z danger:GitHubAPI Sending:  https://api.github.com/repos//iphone-app/pulls/6730/comments { 'Content-Type': 'application/json',
  Authorization: 'token ' }
2019-01-18T15:58:01.368Z danger:GitHubAPI getNextPageFromLinkHeader:: Given response does not contain link header for pagination
2019-01-18T15:58:01.369Z danger:GitHub::Issue Creating inline comment. Commit: 779cfd263ef1f9a67232fe90ce052e99801d29ae
2019-01-18T15:58:01.369Z danger:GitHub::Issue Finding position for inline comment.Users/travis/build//iphone-app/Shared/Constants/Constants.swift#18
2019-01-18T15:58:01.369Z danger:GitHubAPI Sending:  https://api.github.com/repos//iphone-app/pulls/6730 { 'Content-Type': 'application/json',
  Accept: 'application/vnd.github.v3.diff',
  Authorization: 'token ' }
2019-01-18T15:58:01.370Z danger:GitHub::Issue Creating inline comment. Commit: 779cfd263ef1f9a67232fe90ce052e99801d29ae
2019-01-18T15:58:01.370Z danger:GitHub::Issue Finding position for inline comment.Users/travis/build//iphone-app/Shared/Constants/Constants.swift#19
2019-01-18T15:58:01.370Z danger:GitHubAPI Sending:  https://api.github.com/repos//iphone-app/pulls/6730 { 'Content-Type': 'application/json',
  Accept: 'application/vnd.github.v3.diff',
  Authorization: 'token ' }
2019-01-18T15:58:01.371Z danger:GitHub::Issue Creating inline comment. Commit: 779cfd263ef1f9a67232fe90ce052e99801d29ae
2019-01-18T15:58:01.371Z danger:GitHub::Issue Finding position for inline comment.Users/travis/build//iphone-app/Shared/Constants/Constants.swift#21
2019-01-18T15:58:01.372Z danger:GitHubAPI Sending:  https://api.github.com/repos//iphone-app/pulls/6730 { 'Content-Type': 'application/json',
  Accept: 'application/vnd.github.v3.diff',
  Authorization: 'token ' }
2019-01-18T15:58:01.497Z danger:GitHub::Issue Diff found for inline comment, now getting a position.Users/travis/build//iphone-app/Shared/Constants/Constants.swift#19. Diff: null
2019-01-18T15:58:01.565Z danger:GitHub::Issue Diff found for inline comment, now getting a position.Users/travis/build//iphone-app/Shared/Constants/Constants.swift#21. Diff: null
2019-01-18T15:58:01.733Z danger:GitHub::Issue Diff found for inline comment, now getting a position.Users/travis/build//iphone-app/Shared/Constants/Constants.swift#18. Diff: null
2019-01-18T15:58:01.733Z danger:GitHubAPI Sending:  https://api.github.com/user { 'Content-Type': 'application/json',
  Authorization: 'token ' }
2019-01-18T15:58:01.842Z danger:GitHubAPI getPullRequestCommits:: Sending pull request commit request for first page
2019-01-18T15:58:01.842Z danger:GitHubAPI getPullRequestCommits:: Request url generated "repos//iphone-app/issues/6730/comments"
2019-01-18T15:58:01.842Z danger:GitHubAPI Sending:  https://api.github.com/repos//iphone-app/issues/6730/comments { 'Content-Type': 'application/json',
  Authorization: 'token ' }
2019-01-18T15:58:02.082Z danger:GitHubAPI getNextPageFromLinkHeader:: Given response does not contain link header for pagination
2019-01-18T15:58:02.083Z danger:GitHubAPI User ID: 46794206
2019-01-18T15:58:02.083Z danger:GitHubAPI Looking at 3 comments for DangerID: danger-id-default;
2019-01-18T15:58:02.083Z danger:GitHub::Issue Creating new comment
2019-01-18T15:58:02.083Z danger:GitHubAPI Sending:  https://api.github.com/repos//iphone-app/issues/6730/comments { 'Content-Type': 'application/json',
  Authorization: 'token ' }
Feedback: https://github.com//iphone-app/pull/6730#issuecomment-455595139
2019-01-18T15:58:03.658Z danger:GitHubAPI Sending:  https://api.github.com/repos//iphone-app/statuses/779cfd263ef1f9a67232fe90ce052e99801d29ae { 'Content-Type': 'application/json',
  Authorization: 'token ' }
Launching Danger Swift ci (v1.1.0)
Finding out where the danger executable is
Running: /usr/local/bin/danger ci --process danger-swift --passURLForDSL 
The command "DEBUG=* danger-swift ci" exited with 0.

@f-meloni
Copy link
Member

Thank you,
Looks that danger-swift is passing back the correct values, then if there is a problem is probably on danger-js

@steven-diaz
Copy link
Author

(posted from wrong account)

Thanks for investigating!

@absolute-heike
Copy link
Contributor

I'm having the same issues. Haven't had a look at danger-js yet, but looking at:

warnings:
   [ { message:
        'Colons should be next to the identifier when specifying a type and next to the key in dictionary literals. (`colon`)',
       file:
        'Users/travis/build//iphone-app/Shared/Constants/Constants.swift',
       line: 19 },
     { message:
        'Colons should be next to the identifier when specifying a type and next to the key in dictionary literals. (`colon`)',
       file:
        'Users/travis/build//iphone-app/Shared/Constants/Constants.swift',
       line: 21 },
     { message:
        'Limit vertical whitespace to a single empty line. Currently 5. (`vertical_whitespace`)',
       file:
        'Users/travis/build//iphone-app/Shared/Constants/Constants.swift',
       line: 18 } ] }

Could it be a problem, that swiftlint returns absolute paths for the files? Are these absolute paths stripped by danger-js?

@f-meloni
Copy link
Member

@absolute-heike thank you for noticing it (I didn't :D), I think you are right, the swiftlint plugin is supposed to delete the current path.

https://github.com/danger/swift/blob/master/Sources/Danger/Plugins/SwiftLint/SwiftLint.swift#L79

Will investigate on why, also because the path looks odd Users/travis/build//iphone-app/

@absolute-heike
Copy link
Contributor

absolute-heike commented Jan 25, 2019

@f-meloni Also found that code by now :) Though I haven't figured out, whats going wrong there.

I think the full path is definitely the reason. When the warning is printed in our PR it has the full path in it, therefore it cannot inline on GitHub:

drone/src/github.com/company/repo/Dangerfile.swift#L216 - Limit vertical whitespace to a single empty line. Currently 2. (vertical_whitespace)

I think the path in the output above looks off, because @steven-diaz just scrubbed the repo name ;)

@absolute-heike
Copy link
Contributor

absolute-heike commented Jan 25, 2019

Here is my output for reference:

danger:executor Writing to STDOUT: { messages: [ { message: 'Jira tickets: none' } ],
  markdowns: [],
  meta:
   { runtimeHref: 'https://danger.systems',
     runtimeName: 'Danger Swift' },
  fails: [],
  warnings:
   [ { message:
        'Limit vertical whitespace to a single empty line. Currently 2. (`vertical_whitespace`)',
       file: 'Users/My.Name/path/to/repo/Dangerfile.swift',
       line: 216 } ] } +1ms

Swiftlint output

> swiftlint lint --quiet --reporter json --path "Dangerfile.swift"
[
  {
    "character" : null,
    "file" : "\/Users\/My.Name\/path\/to\/repo\/Dangerfile.swift",
    "line" : 216,
    "reason" : "Limit vertical whitespace to a single empty line. Currently 2.",
    "rule_id" : "vertical_whitespace",
    "severity" : "Warning",
    "type" : "Vertical Whitespace"
  }
]

@absolute-heike
Copy link
Contributor

absolute-heike commented Jan 25, 2019

Found out that https://github.com/danger/swift/blob/master/Sources/Danger/Plugins/SwiftLint/SwiftLint.swift#L135 is exiting early. But it's very weird 🤨

hasPrefix failed | string: /Users/My.Name/path/to/repo/Dangerfile.swift | prefix: /Users/My.Name/path/to/repo

The deletingPrefix extension is only failing, when I get the input from danger.utils.exec. When I just copy the input strings and do everything in a playground, everything is fine...

@absolute-heike
Copy link
Contributor

Found it! 🎉

https://github.com/danger/swift/blob/master/Sources/Danger/Plugins/SwiftLint/CurrentPathProvider.swift#L9

ShellExecutor().execute("pwd") includes a newline at the end. I can open a PR for this ;)

@f-meloni
Copy link
Member

@absolute-heike thanks :) PRs are more than welcome

@absolute-heike
Copy link
Contributor

PR is open :)

@f-meloni
Copy link
Member

Fixed by #179

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants