nikitasavinov / checkstyle-action

Run Java checkstyle with reviewdog in github actions
MIT License
59 stars 37 forks source link

The Pipeline does not fail with 'fail_on_error: true' #24

Closed SimonHarmonicMinor closed 3 years ago

SimonHarmonicMinor commented 3 years ago

I added yml configuration according to the guideline.

name: "Run Java Checkstyle"

on: pull_request

jobs:
  checkstyle_job:
    runs-on: ubuntu-latest
    name: Checkstyle job
    steps:
      - name: Checkout
        uses: actions/checkout@v2
      - name: Run check style
        uses: nikitasavinov/checkstyle-action@master
        with:
          github_token: ${{ secrets.GITHUB_TOKEN }}
          reporter: 'github-pr-check'
          checkstyle_config: 'codestyle/checkstyle.xml'
          checkstyle_version: '8.42'
          fail_on_error: true

Even that fail_on_error is set to true, the checkstyle pipeline finishes successfully. Here is the link to PR: SimonHarmonicMinor/Java-Useful-Utils#72

alexcosmin commented 3 years ago

We're seeing the same behaviour here with fail_on_error: true . The action still passes.

nikitasavinov commented 3 years ago

Hi @SimonHarmonicMinor , There are no code issues in the PR you attached (https://github.com/SimonHarmonicMinor/Java-Useful-Utils/actions/runs/954371421) as there is no Java code. Do you have another example maybe?

In the sample PR I see that fail_on_error option works: https://github.com/nikitasavinov/checkstyle-action/pull/6/files, so I need some help to reproduce

SimonHarmonicMinor commented 3 years ago

Hi @nikitasavinov . Here it is https://github.com/SimonHarmonicMinor/Java-Useful-Utils/pull/133. The plugin reported an error, but the job finished successfully.

nikitasavinov commented 3 years ago

Ok, I see: you need to provide an extra level flag in your build definition:

level: error

In that case all reviewdog messages will become errors and build will fail (reference: https://github.com/reviewdog/reviewdog#exit-codes). Not very obvious, I will update the documentation.

The current default value is info.

SimonHarmonicMinor commented 3 years ago

@nikitasavinov thanks, that did work. I think this should be mentioned in README.md