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

fail() does not fail GitLab job #964

Open
mbogh opened this issue Mar 6, 2018 · 16 comments
Open

fail() does not fail GitLab job #964

mbogh opened this issue Mar 6, 2018 · 16 comments

Comments

@mbogh
Copy link
Contributor

mbogh commented Mar 6, 2018

Report

What did you do?

  1. Call fail("Please include a CHANGELOG entry. \nYou can find it at [CHANGELOG.md](https://..../blob/master/CHANGELOG.md).", sticky: false) in my Dangerfile
  2. Job succeeds
Running with gitlab-ci-multi-runner 9.2.0 (ABCDEF)
  on XXX (ABCDEF)
Using Shell executor...
Running on XXX...
Fetching changes...
Removing .ruby-version
Removing DerivedData/
Removing fastlane/report.xml
Removing vendor/
HEAD is now at ABC Blue status bar background is now reintroduced
From https://...
   ABC..DEF  some-branch -> origin/some-branch
Checking out ABC-DEF as some-branch...
Skipping Git submodules setup
Downloading artifacts for Prepare (397017)...
Downloading artifacts from coordinator... ok        id=XXX responseStatus=200 OK token=XXX
$ rbenv local 2.4.2
$ xcversion select 9.2
$ export CI_MERGE_REQUEST_ID=$(git ls-remote -q origin merge-requests\*\head | grep $CI_COMMIT_SHA | sed 's/.*refs\/merge-requests\/\([0-9]*\)\/head/\1/g')
$ bundle exec danger --danger_id=danger --dangerfile=danger/Dangerfile --new-comment
Results:

Errors:
- [ ] Please include a CHANGELOG entry. 
You can find it at [CHANGELOG.md](https://.../blob/master/CHANGELOG.md).
Job succeeded

What did you expect to happen?

The output as above but with Job failed as the final message.

What happened instead?

Job succeeded

Your Environment

  • GitLab CI

  • Danger 5.5.10

  • What is your Dangerfile?

    if !git.modified_files.include?("CHANGELOG.md")
      fail("Please include a CHANGELOG entry. \nYou can find it at [CHANGELOG.md](https://.../blob/master/CHANGELOG.md).", sticky: false)
    end
@mbogh
Copy link
Contributor Author

mbogh commented Mar 6, 2018

So I have an idea on how to fix this.

Currently update_pull_request in gitlab.rb just ends by updating the Merge Request.
If we add something like this at the end of that method the job would fail instead of succeeding.

  raise "Errors #{errors}" unless errors.empty?

@orta
Copy link
Member

orta commented Mar 6, 2018

This might be on purpose, if danger can successfully set a error status on the PR it won't fail the build I think

@mbogh
Copy link
Contributor Author

mbogh commented Mar 6, 2018

The MR status is determined from the pipeline status, so if the pipeline succeeds. Then the MR is considered green.

The API docs does not mention of a way of failing a MR.

@orta
Copy link
Member

orta commented Mar 6, 2018

Makes sense, I feel like the --fails-on-errors flag should probably default to true on GitLab then 👍

@mbogh
Copy link
Contributor Author

mbogh commented Mar 6, 2018

pipeline

The Danger step here actually had errors.

@mbogh
Copy link
Contributor Author

mbogh commented Mar 6, 2018

D'oh I missed that flag.

@joshfriend
Copy link

--fail-on-errors=true is not currently working (danger 6.0) on gitlab

@eric-ludlow-ios
Copy link

@here
Is there any update yet about a solution or resolution of this issue?

@orta
Copy link
Member

orta commented Sep 18, 2019

Doesn't look like it, you're welcome to take a look

@MiralDesai
Copy link

I'm guessing this can be closed, I'm using v6.3.1 and it works with our self hosted gitlab.

@eric-ludlow-ios
Copy link

eric-ludlow-ios commented Apr 1, 2020

I will update to version 6.3.1 and post back my results. It may take a few days.

@joshfriend
Copy link

I just tested v6.3.2 and no it still does not work on gitlab

@MiralDesai
Copy link

MiralDesai commented Apr 3, 2020

If it helps debug I'm using v12.8.6-ee of gitlab with v6.3.1 of danger where I see my pipeline failing

@joshfriend
Copy link

@MiralDesai I'm running Gitlab 12.9.2-ee with danger 6.3.2. Perhaps you could share bits of your config and how you are invoking danger from ci?

In my .gitlab-ci.yml I invoke danger with:

bundle exec danger --fail-on-errors=true

And here's my Dangerfile:

kotlin_detekt.gradle_task = "detekt"
kotlin_detekt.report_file = "./build/reports/detekt.xml"
kotlin_detekt.severity = "warning"
kotlin_detekt.detekt

android_lint.gradle_task = "lintDebug"
android_lint.report_file = "./app/build/reports/lint-results-debug.xml"
android_lint.lint

Danger runs successfully, posts the results of the build in a comment to the MR, but does not fail the pipeline.

@MiralDesai
Copy link

MiralDesai commented Apr 3, 2020

Sure thing.

My .gitlab-ci.yml

script:
    - bundle exec danger --fail-on-errors=true
  only:
    - branches
  except:
    - master

and my Dangerfile

# # --- Only a max of 2 commits allowed.
if git.commits.count > 2
   fail("Please squash your commits into 2 commits or less", sticky: true)
end

# # --- All MRs must have description
if gitlab.mr_body.length < 5
   fail("Please proivde a summary for this change", sticky: true)
end

And here are the last few lines when I run my pipeline:

$ bundle exec danger --fail-on-errors=true
Results:
Errors:
- [ ] Please squash your commits into 2 commits or less
- [ ] Please proivde a summary for this change
ERROR: Job failed: exit code 1

I experienced the same thing you did before I added --fail-on-errors, the pipeline would succeed.

@joshfriend
Copy link

I think what might be happening is that the kotlin-detekt and android lint plugins don't call fail() after finding errors. I had assumed that generating errors to be reported in a comment would be considered a build failure, but i guess that's not true?

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

5 participants