Skip to content

Unexpected error exit code when using -reporter=github-pr-check with -fail-on-error=true #1846

@Drowze

Description

@Drowze

In our CI, we're running reviewdog with the following command:

reviewdog -conf=.reviewdog.yml -reporter=github-pr-check -runners=Rubocop,Brakeman,Cspell -fail-on-error=true

And our .reviewdog.yml looks like the following:

runner:
  pronto-rubocop:
    cmd: bundle exec pronto run --commit origin/${PR_TARGET_BRANCH:-develop} --runner rubocop
    errorformat:
      - "%A%f:%l %t: %m"
      - "%Z```%$"
      - "%C%.%#"
    name: Rubocop
    level: error

  pronto-brakeman:
    cmd: bundle exec pronto run --commit origin/${PR_TARGET_BRANCH:-develop} --runner brakeman
    errorformat:
      - "%f:%l %t: %m"
    name: Brakeman
    level: warning

  cspell:
    cmd: npx cspell lint ${PR_CHANGED_FILES} --config .cspell.yml --relative --no-summary --no-progress --no-must-find-files --gitignore || true
    errorformat:
      - "%f:%l:%c - %m"
    name: Cspell
    level: warning

With that setup, the behavior on v0.18.1:

  • whenever we had an infraction on rubocop, we'd get an error exit code
  • whenever we didn't have an infraction on rubocop, we'd get a success exit code (regardless of cspell/brakeman infractions)
    Screenshot 2024-07-15 at 09 40 14

With that setup, the behavior on >=v0.20.0:

  • whenever we have an infraction on any runner, we get an error exit code
    Screenshot 2024-07-15 at 09 44 51

Probably a related buggy behaviour:

Following the same setup, on >=v0.20.0:

  • when -fail-on-error=true: all the checks are reported to GitHub were reported with the default name reviewdog instead of respecting the check name defined in the config file. And even more interesting, all the checks were failed cspell checks (there wasn't any rubocop or brakeman checks reported in my tests - where those checks should be green)
  • when -fail-on-error=false: only the cspell red check was reported to GitHub, and it was named with the default name reviewdog instead of respecting the custom name defined in the config file.

In respect to the -fail-on-error flag, I was a bit confused by this part of the README:

Exit codes

By default reviewdog will return 0 as exit code even if it finds errors. If -fail-on-error flag is passed, reviewdog exits with 1 when at least one error was found/reported. This can be helpful when you are using it as a step in your CI pipeline and want to mark the step failed if any error found by linter.
See also -level flag for github-pr-check/github-check reporters. reviewdog will exit with 1 if reported check status is failure as well if -fail-on-error=true.

My interpretation from that is, when using the github-pr-check reporter alongside the -fail-on-error flag, we should only get an error exit code if both of the following are satisfied:

  • there was at least one infraction from any runner
  • there was a failure check reported to GitHub API

So considering the interpretation above, I guess the new behavior is a bug (but would like to hear an input from @haya14busa to confirm)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions