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

Failed Assertion run_action.py:244 #47

Closed
johnboiles opened this issue Jun 30, 2023 · 1 comment · Fixed by #48
Closed

Failed Assertion run_action.py:244 #47

johnboiles opened this issue Jun 30, 2023 · 1 comment · Fixed by #48

Comments

@johnboiles
Copy link

johnboiles commented Jun 30, 2023

My repo is failing this assertion

assert all(path == file_paths[0] for path in file_paths)
My Workflow yml
name: idf

on:
  pull_request:
  push:
    branches:
      - main

jobs:
  clang-tidy:
    runs-on: ubuntu-latest
    container:
      image: espressif/idf:release-v5.1
    steps:
      - uses: actions/checkout@v2
        with:
          submodules: true

      - name: Install tools
        run: |
          . $IDF_PATH/export.sh
          idf_tools.py install esp-clang
          pip install --upgrade pyclang

      - name: Run clang-tidy
        run: |
          . $IDF_PATH/export.sh
          idf.py clang-check --run-clang-tidy-options "-export-fixes build/fixes.yml -header-filter=main/.*" --exclude-paths managed_components

      - name: Run clang-tidy-pr-comments action
        uses: platisd/clang-tidy-pr-comments@master
        with:
          github_token: ${{ secrets.GITHUB_TOKEN }}
          clang_tidy_fixes: build/fixes.yml
          request_changes: true
          suggestions_per_comment: 10
          repo_path_prefix: "/__w"

      - uses: actions/upload-artifact@v3
        if: always()
        with:
          name: fixes
          path: build/fixes.yml
My build/fixes.yml (`head -n 50` -- full version attached)
Diagnostics:
- BuildDirectory: /__w/mainframe-tester/mainframe-tester/build
  DiagnosticMessage:
    FileOffset: 95
    FilePath: /__w/mainframe-tester/mainframe-tester/main/GPIO.hpp
    Message: invalid case style for global constant 'kDut3_3V'
    Replacements:
    - FilePath: /__w/mainframe-tester/mainframe-tester/main/GPIO.hpp
      Length: 8
      Offset: 95
      ReplacementText: kDut33V
    - FilePath: /__w/mainframe-tester/mainframe-tester/main/GPIO.cpp
      Length: 8
      Offset: 82
      ReplacementText: kDut33V
  DiagnosticName: readability-identifier-naming
  Level: Warning
- BuildDirectory: /__w/mainframe-tester/mainframe-tester/build
  DiagnosticMessage:
    FileOffset: 1481
    FilePath: /__w/mainframe-tester/mainframe-tester/main/GPIO.hpp
    Message: invalid case style for global function 'initGpio'
    Replacements:
    - FilePath: /__w/mainframe-tester/mainframe-tester/main/GPIO.hpp
      Length: 8
      Offset: 1481
      ReplacementText: InitGpio
    - FilePath: /__w/mainframe-tester/mainframe-tester/main/GPIO.cpp
      Length: 8
      Offset: 46
      ReplacementText: InitGpio
  DiagnosticName: readability-identifier-naming
  Level: Warning
- BuildDirectory: /__w/mainframe-tester/mainframe-tester/build
  DiagnosticMessage:
    FileOffset: 35
    FilePath: /__w/mainframe-tester/mainframe-tester/main/NVS.hpp
    Message: invalid case style for global function 'initNvs'
    Replacements:
    - FilePath: /__w/mainframe-tester/mainframe-tester/main/NVS.hpp
      Length: 7
      Offset: 35
      ReplacementText: InitNvs
  DiagnosticName: readability-identifier-naming
  Level: Warning
- BuildDirectory: /__w/mainframe-tester/mainframe-tester/build
  DiagnosticMessage:
    FileOffset: 52
    FilePath: /__w/mainframe-tester/mainframe-tester/main/NVS.hpp
    Message: invalid case style for global function 'swap_servo_direction'

fixes.yml.zip

Nothing seems out of place in my paths. The __w part is because I'm indeed running clang-tidy inside docker, but seems like that is what the repo_path_prefix is meant to handle.

I added a simple bit of printing to my fork before the assert:

for path in file_paths:                
    print(f"{path} == {file_paths[0]}")

And I see this in the logs:

main/mainframe-tester.cpp == main/mainframe-tester.cpp
main/ADC.hpp == main/mainframe-tester.cpp

So it would seem that the assertion failure is coming from this replacement:

- BuildDirectory: /__w/mainframe-tester/mainframe-tester/build
  DiagnosticMessage:
    FileOffset: 5
    FilePath: /__w/mainframe-tester/mainframe-tester/main/ADC.hpp
    Message: invalid case style for global function 'initAdc'
    Replacements:
    - FilePath: /__w/mainframe-tester/mainframe-tester/main/mainframe-tester.cpp
      Length: 7
      Offset: 4544
      ReplacementText: InitAdc
    - FilePath: /__w/mainframe-tester/mainframe-tester/main/ADC.hpp
      Length: 7
      Offset: 5
      ReplacementText: InitAdc
  DiagnosticName: readability-identifier-naming
  Level: Warning

This replacement seems valid, since it's changing a method name in both the header where it's defined and in a .cpp file where it's used.

It appears the code in its current state assumes that each replacement is only in a single file, which seems to not necessarily be the case, at least in clang-tidy 15.

@platisd
Copy link
Owner

platisd commented Jun 30, 2023

I can't take a look right away but it sounds like it's the same as #46 unfortunately and it will eventually have to be fixed. :/

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

Successfully merging a pull request may close this issue.

2 participants