"Not a Pull Request - skipping `danger` run"

[KrauseFx]

I submitted a PR via the GitHub web UI (https://github.com/Themoji/ios/pull/14) and danger 0.7.3 thinks that it's not a PR: https://circleci.com/gh/Themoji/ios/91

bundle exec danger Not a Pull Request - skipping danger run

[joshrlesch]

having same experience here also. And I also only execute the danger command if the CI server sees a PR is open for the branch.

[orta]

Interesting, I wonder if this is something inherit to what circle CI thinks, there might be a GitHub API to look up if a commit is in an active PR and we could use that as a fallback when there's no env vars?

[orta]

likely related https://github.com/danger/danger/issues/226

[joshrlesch]

Any progress on this?

[orta]

You're welcome to take a look at #226 which should deal with this

[orta]

#292 should fix this

[KrauseFx]

Still have that with the most recent release for fastlane: https://circleci.com/gh/fastlane/fastlane/6135 😢

[orta]

Did you go through the getting set up on Circle CI part of the guide? http://danger.systems/guides/getting_started.html

[therealbnut]

I just got this message on our CI (Buildkite), I believe it happened because it was the first and only commit on a PR, CI ran when the commit went up, before the PR was opened.

Not a Pull Request - skipping danger run

I think a desirable fix would be to make danger exit with a non-zero code when it cannot detect a PR. That way when the PR is open it cannot be merged without running danger, you can re-run the build, or trigger it with other commits etc.

[orta]

There are CI services that run both for a PR and a commit on every PR commit - that would mean you need determine when to run danger when integrating yourself. As well as handling the CI for when it's merged into master etc.

IMO, it doesn't need to be fixed.

[mandrizzle]

This is happening to me on Circle CI 2.0 for my pull requests. It was working fine on 1.0, but since switching, Danger has been working sometimes despite having all the required environment variables set.

[ppaulojr]

It happens randomly for me.

[doodla]

Still happens randomly on Circle 2.0 for me.

[Startouf]

Could it be that it happens if the test started to run before the PR was created ?

[therealbnut]

@Startouf yes, for me at least that's exactly the reason. IMO danger should return a non-zero exit code in this case.

[orta]

There's an option for that I think. A bunch of CIs do both a merge branch and a PR branch, it would fail on the merge branch every time if that was the default.

[NachoSoto]

I just switched from danger-js (see https://github.com/danger/danger-js/issues/435) and this is happening to me too on CircleCI 2.0.

[therealbnut]

I still think build failure is the right choice, but @orta’s suggestion of rebuilding on the “PR open” hook worked for me (edit: on Buildkite).

[therealbnut]

@levibostian sorry, I should have clarified, I'm not using CircleCI.

There may be a setting for it in CircleCI (I'm not familiar sorry), but until you have a better solution you may be able to get something like this (untested) to force a CI failure when there isn't a PR:

if [[ -z "${CI_PULL_REQUESTS}" ]]; then
  &2> echo -e "\033[31merror: Danger requires a pull request to function correctly\033[0m"
  exit 1
fi

[therealbnut]

For Github in your project settings:

There should be a web hook already for your CI:

The Pull Request hook is the one you'll need, if CircleCI supports it:

edit: @levibostian if you have further questions about setting up web hooks and running builds when a pull request is open you may get more support from the CircleCI forums, but please submit your findings here :)

[therealbnut]

@levibostian actually, it looks like there's fairly comprehensive instructions in the danger docs: http://danger.systems/guides/getting_started.html (search for CircleCI, I can't directly link you sorry, it'd be nice if http://danger.systems/guides/getting_started.html#circleci took you there)

[levibostian]

@therealbnut Thank you very much.

It appears that Circle is not designed in a way to work nicely for Danger. Circle does not trigger a new build when a PR is created on GitHub (I have the pull request hook enabled on the CircleCI webhook as you mention above).

Circle is also not very reliable I can tell in checking if a commit is part of a pull request. Circle can tell you if a branch is part of a PR, but not the commit you pushed up. This gets to your comment about the getting started guide. It suggests to setup Circle to only build when you make a PR instead of on commit. That's not satisfactory to me.

---

I am going to go back to the drawing board a bit. I want to use Danger, but if Circle does not support it very reliably, I might as well use a pull request template until Circle adds better support for Danger.

[therealbnut]

@levibostian sounds fair, I had to wait for PR metadata support in Buildkite. You can make a GitHub API call to determine if the PR exists. Danger does this call already afaik, but unfortunately the Danger command still succeeds when there isn't a PR.

Read the next section in the Danger docs, it talks about triggering from a commit as well as the PR, it may be worth trying.

[levibostian]

I found a way to make CircleCI and Danger work OK together thanks to @therealbnut's suggestion.

In my project, I created the following bash script named assert_danger_running_on_pr.sh in the root directory of my project. Here is the content of the file:

# This file exists because CircleCI does not have very good support (yet) for Danger.
#
# When you create a commit on github, CircleCI triggers a build of that commit, which could include running Danger.
# When you run Danger, it checks if your *branch* currently on is part of *at least 1* pull request on GitHub. *Not* if that *commit* is part of *a pull request*.
# Also, if you were to push up a commit to GitHub, then create a pull request, CircleCI does not trigger *another* build of your project from that pull request. This is why
# Danger recommends that you only setup CircleCI to build on pull requests instead of commits (access through project Advanced Settings in CircleCI). But, I like running tasks
# on commits so I do not like that option.
#
# What this script does is checks if this *branch* is part of *a pull request* (the best we can do at the moment). If not, throw an error.
# This asserts that when we create a commit and then create a pull request, Danger will be marked as failed instead of as passed. So when we create a new pull request, we
# need to go into CircleCI and manually trigger a rebuild of the Danger task once the pull request is created and it will run successfully.
# Sure, this sucks but it's better then Danger passing as `exit 0` code when the pull request is not created yet, you create the pull request, see Danger ran "successfully" (but not really, just passed)
# and then you merged in your pull request without Danger actually running. This is as good as we can do until CircleCI improves for us.

if [[ -z "${CI_PULL_REQUESTS}" ]]; then
  echo -e "\033[31merror: Danger requires a pull request to function correctly\033[0m"
  exit 1
fi

Note: Make sure to make this script executable after creating: chmod u+x assert_danger_running_on_pr.sh

Then, inside of my .circleci/config.yml file, I added the execution of this bash script before I run danger:

  danger:
    docker:
      - image: dantoml/danger:latest
    steps:
      - checkout
      - run: ./assert_danger_running_on_pr.sh
      - run: bundle install
      - run: '[ ! -z $DANGER_GITHUB_API_TOKEN ] && danger || echo "Skipping Danger for External Contributor" && exit 1'

Not very pretty, but I am happy with it for the time being until CircleCI adds better support for PRs.

[hjhart]

@levibostian I found your post very helpful, and I might implement danger into our project this exact way.

Is this still the generally what is recommended for CircleCI and Danger? Thanks in advance.

[Rasbcyberassn42]

Sorry, but i actually not understance what kind of post that you mean it.. Can you explain to me.

R©ASB

On Tue, Jun 4, 2019, 14:51 James Hart [email protected] wrote:

@levibostian https://github.com/levibostian I found your post very helpful, and I might implement danger into our project this exact way.

Is this still the generally what is recommended for CircleCI and Danger? Thanks in advance.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/danger/danger/issues/204?email_source=notifications&email_token=AMBUU577UXC63JBFYYE4WCTPYYGHHA5CNFSM4CC6TXYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODW3TC5Q#issuecomment-498545014, or mute the thread https://github.com/notifications/unsubscribe-auth/AMBUU546OYUUDFO5XJSAGM3PYYGHHANCNFSM4CC6TXYA .

[levibostian]

@hjhart, sorry, but I am not using Circle much these days. It's expensive macOS pricing and it's not so good pull request support outlined in this issue got me to use travis more and more. At the end of the day, CI servers are all just tools. It doesn't matter what one you use. Use what works best for you.

That I know of, this is still the preferred way of working with danger and Circle. I have not read otherwise.

[hjhart]

If anyone else runs into this problem, there is a new command line argument called —fail-if-not-Pr=true

[pabsanmez]

If anyone else runs into this problem, there is a new command line argument called —fail-if-not-Pr=true

The correct argument is --fail-if-no-pr=true

[alejandrovelez7]

@pabsanmez is that in the docs somewhere?