github-actions-analyze-dart icon indicating copy to clipboard operation
github-actions-analyze-dart copied to clipboard

Action not working properly inside example/

Open llucax opened this issue 5 years ago • 14 comments

Hi, I have a package that provides an example. When running flutter analyze locally, it all works fine, but when running this GitHub action I get some errors. See: https://github.com/lunofono/lunofono_player/pull/9/checks?check_run_id=1727456103#step:6:18

At some point I had those errors locally, but it was fixed after doing a flutter pub get, so maybe the action is missing that step?

llucax avatar Jan 19 '21 11:01 llucax

At some point I had those errors locally, but it was fixed after doing a flutter pub get, so maybe the action is missing that step?

The weird thing about this is none of the pubspec.lock files were changed (in the root or in the example/) by the pub get.

llucax avatar Jan 19 '21 11:01 llucax

@llucax, I'll check this out further tomorrow. The readme already shows examples to run flutter pub get or dart pub get, or did you mean it also needs to be run inside of your examples folder?

zgosalvez avatar Jan 19 '21 12:01 zgosalvez

@llucax, I'll check this out further tomorrow. The readme already shows examples to run flutter pub get or dart pub get, or did you mean it also needs to be run inside of your examples folder?

Yes, I'm running flutter pub get in the top folder. Not sure if it is necessary to run it somewhere else (it is supposed to run inside example too automatically when doing it in the top-level). I'm not exactly sure why this is failing in the action and not locally, nor how could I reproduce it locally, so I'm sorry I can give so few details. I can't spend much time on debugging this myself now, but maybe you had an idea on what the cause could be. If I manage to do some more investigation I will share my findings here.

llucax avatar Jan 19 '21 12:01 llucax

See: https://github.com/lunofono/lunofono_player/pull/9/checks?check_run_id=1727456103#step:6:18

@llucax, this link no longer points to the issue. Please identify which run failed here: https://github.com/lunofono/lunofono_player/actions. I want to be sure of the error first before I attempt to debug.

zgosalvez avatar Jan 20 '21 12:01 zgosalvez

I created a separate PR that only upgrades to this action, so it's easier to see the issue in isolation: https://github.com/lunofono/lunofono_player/pull/12

llucax avatar Jan 20 '21 13:01 llucax

Great! I appreciate the help. I'll look into this in the coming days.

zgosalvez avatar Jan 20 '21 13:01 zgosalvez

The job using the action is test (static), here is a direct link to the failure again: https://github.com/lunofono/lunofono_player/pull/12/checks?check_run_id=1730340592#step:6:14

llucax avatar Jan 20 '21 14:01 llucax

@llucax, I found that the issue is caused by the dartanalyzer running from the top-level project, since it is also analyzing the example directory, which is technically a different project. I got it to work by splitting the jobs like so:

    - name: Analyze lib
      if: matrix.test_type == 'static'
      uses: zgosalvez/github-actions-analyze-flutter@v1
      with:
        working-directory: lib
        fail-on-warnings: true

    - name: Analyze test
      if: matrix.test_type == 'static'
      uses: zgosalvez/github-actions-analyze-flutter@v1
      with:
        working-directory: test
        fail-on-warnings: true

    - name: Analyze example
      if: matrix.test_type == 'static'
      uses: zgosalvez/github-actions-analyze-flutter@v1
      with:
        working-directory: example
        fail-on-warnings: true

I could also implement an excludes input to this action so that we can simplify the workflow further like so:

    - name: Analyze
      if: matrix.test_type == 'static'
      uses: zgosalvez/github-actions-analyze-flutter@v1
      with:
        excludes: example
        fail-on-warnings: true

    - name: Analyze example
      if: matrix.test_type == 'static'
      uses: zgosalvez/github-actions-analyze-flutter@v1
      with:
        working-directory: example
        fail-on-warnings: true

Thoughts?

zgosalvez avatar Jan 21 '21 03:01 zgosalvez

For me what would be natural as a user of the action is that these details are handled automatically and the action behaves the same as running flutter analyze which handles the change of directory correctly when analyzing the example directory, as example is a well known standard directory in a flutter/dart package.

llucax avatar Jan 21 '21 07:01 llucax

It appears this action should be refactored to use the dart tool instead of using dartanalyzer and dartfmt to analyze the code. This will support your use case. Since this will be a major change to the functionality, I'll take a few weeks to have this done and tested on my projects. My main hurdle would be parsing the dart tool's response.

zgosalvez avatar Jan 23 '21 03:01 zgosalvez

Oh, dann, that seems like a lot of work. I would have thought that the dart tool commands used dartanalyzer et al and had the same output format. Thanks for the update. I might go with the workaround for now

llucax avatar Jan 23 '21 09:01 llucax

I had initially thought that as well when I developed this action. When I read the dart tool documentation, it mentioned using the dart tool instead of dartanalyzer and dartfmt. Doesn't explain much but it seems that they're planning to have dart replace most of the existing commands.

zgosalvez avatar Jan 23 '21 10:01 zgosalvez

Hey is there any update on this? the exclude option in analysis_options.yaml doesn't seem to be respected

analyzer:
  exclude: [dirA, dirB]
linter:
  rules:
#    flutter
    - avoid_print
    - ....

Bhupesh-V avatar Oct 29 '21 08:10 Bhupesh-V

I've been working around this for a while, but just happened to see this issue. If anyone is curious I just override dartanalyzer with my own script which also reformats the output since flutter analyze outputs in a different format than what the annotation expects:

    - name: Hack to use flutter analyze instead of dartanalyzer
      run: |
        cat >/opt/flutter/bin/dartanalyzer <<EOF
          #!/bin/bash
          flutter analyze --no-preamble --no-pub | head -n-1 | sed -r 's/\s[\x80-\xFF]+\s/|/g; s/^\s+//; s/:/|/g;' | awk -F\| '{printf "%s||%s|%s|%s|%s||%s\n", toupper(\$1), \$6, \$3, \$4, \$5, \$2}'
        EOF
        chmod +x /opt/flutter/bin/dartanalyzer

jbrownsw avatar Jan 14 '22 04:01 jbrownsw