[BUG] danger ci not running in a gitlab.com MR

AnthonyMastrean commented 4 years ago

Describe the bug Danger doesn't think it's running in a CI environment or a PR context in a gitlab.com MR (I can't quite tell which, DEBUG logging is a little sparse on startup).

Skipping Danger due to this run not executing on a PR.

To Reproduce Steps to reproduce the behavior:

I added a simple Dangerfile


import { message, danger } from "danger"

const modifiedMD = danger.git.modified_files.join("- ") message("Changed Files in this PR: \n - " + modifiedMD)

2. And a basic .gitlab-ci.yml

danger: image: node:lts before_script:

printenv script:
DEBUG="*" npx danger ci
```
3. To a branch and opened an MR
4. Check the pipeline logs (snipped some of the env output)
```
$ printenv ... GITLAB_CI=true CI=true DANGER_GITLAB_API_TOKEN=[MASKED] ... $ DEBUG="*" npx danger ci 2020-04-02T15:34:30.576Z danger:process_runner Debug mode on for Danger v10.0.0 2020-04-02T15:34:30.578Z danger:process_runner Starting sub-process run Skipping Danger due to this run not executing on a PR.

Expected behavior

I hoped that the presence of the CI and GITLAB_CI and other env vars would inform danger that it was running in a GitLab CI environment!

Screenshots If applicable, add screenshots to help explain your problem.

Your Environment

software	version
danger.js	10.0.0
node	12.6.1
npm	6.13.4
Operating System

Additional context Add any other context about the problem here.

orta commented 4 years ago

One of these variables must not be set: https://github.com/danger/danger-js/blob/master/source/ci_source/providers/GitLabCI.ts#L15-L20

I'd check what's in your ENV and compare the two

AnthonyMastrean commented 4 years ago

Oh, the gitlab documentation for the CI_MERGE_REQUEST_IID variable says

The IID of the merge request if the pipelines are for merge requests. Available only if only: [merge_requests] or rules syntax is used and the merge request is created.

https://docs.gitlab.com/ee/ci/variables/predefined_variables.html

That means I need to add this to my .gitlab-ci.yml

only:
  - merge_requests

I could send a PR with updated GitLab docs if you like?

NotMoni commented 4 years ago

@AnthonyMastrean I think that would be good.

unikitty37 commented 4 years ago

~~I still haven't managed to get this to work, and I'm not sure what's going wrong, as CI_MERGE_REQUEST_IID is set and is an integer, and CI_PROJECT_PATH is also set~~ (update: they were set, but not getting passed through to the Docker container correctly) — @AnthonyMastrean, did you get anywhere with the docs PR?

It's odd that CI_MERGE_REQUEST_IID is required, though, as it only gets set if pipelines for merge requests are enabled, and that requires fiddling with .gitlab-ci.yml. Danger for Ruby uses CI_MERGE_REQUEST_IID if it exists, falling back to CI_EXTERNAL_PULL_REQUEST_IID — but if neither exists, it calculates what to use from CI_COMMIT_SHA, which is always available.

The upshot of this is that my Ruby backend project runs Danger right out of the box, and the only thing I needed to do was to ensure the base URL, host and token were passed through. I didn't have to reconfigure the whole pipeline setup to try to make sure GitLab CI would set CI_MERGE_REQUEST_IID, because Danger fell back to something else that was set instead.

It would really be useful if danger-js could work the same way; I'm not sure if there's a specific reason for this, or just the two projects having diverged over time.

mat813 commented 2 years ago

I usually don't have my ci run on merge requests but on branch commits, attached to merge requests, so I never have CI_MERGE_REQUEST_IID. On the other hand, there is a CI_OPEN_MERGE_REQUESTS that contains project/path!id.

I ended up having:

  script:
    - export CI_MERGE_REQUEST_IID=${CI_OPEN_MERGE_REQUESTS#*!}
    - yarn danger ci --failOnErrors

buffcode commented 1 year ago

In addition to @mat813 excellent finding, we are using the following to allow both merge request and branch pipelines with the correct MR:

script:
  - export CI_MERGE_REQUEST_IID="${CI_MERGE_REQUEST_IID:-${CI_OPEN_MERGE_REQUESTS#*!}}"
  - yarn danger ci --failOnErrors

It will use CI_MERGE_REQUEST_IID if set (in MR pipelines), otherwise fall back to CI_OPEN_MERGE_REQUESTS (in other pipelines).

fbartho commented 7 months ago

GitLab users should feel free to contribute a PR adjusting the fallback routes of GitLab’s CI code!

danger / danger-js

[BUG] danger ci not running in a gitlab.com MR #1029