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

Try to fix pre-commit failure #35

Merged
merged 1 commit into from
Feb 27, 2024
Merged

Try to fix pre-commit failure #35

merged 1 commit into from
Feb 27, 2024

Conversation

shenxianpeng
Copy link
Collaborator

No description provided.

Copy link

sonarcloud bot commented Feb 27, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link

codecov bot commented Feb 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.84%. Comparing base (a501e3b) to head (27c7a1d).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #35   +/-   ##
=======================================
  Coverage   77.84%   77.84%           
=======================================
  Files           7        7           
  Lines         176      176           
=======================================
  Hits          137      137           
  Misses         39       39           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@shenxianpeng shenxianpeng changed the title Update test.yml Try to fix pre-commit failure Feb 27, 2024
@shenxianpeng shenxianpeng merged commit 92c61dc into main Feb 27, 2024
7 checks passed
@shenxianpeng shenxianpeng deleted the shenxianpeng-patch-1 branch February 27, 2024 09:39
@shenxianpeng shenxianpeng added the bug Something isn't working label Feb 27, 2024
@shenxianpeng
Copy link
Collaborator Author

It's very curious when running pre-commit job regular fails (it can be passed after I clicked Re-run all jobs), I tried to make changes in this PR to fix it, I know it's not a valid revision, but pre-commit job passed.

After I merged it into the main branch, pre-commit job failed again .

@2bndy5 Do you know what causes it failed?

@2bndy5
Copy link
Contributor

2bndy5 commented Feb 27, 2024

Looks like you're running pytest as a pre-commit hook. I don't think its a good idea to run unit testing in a reusable workflow...

Looking at the logs you linked to, I see the test hook failed.

tests....................................................................Failed

More specifically, the test hook failed to build a wheel for cpp_linter_hooks

  × Building wheel for cpp_linter_hooks (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [23 lines of output]
      /tmp/pip-build-env-gnanr2jk/overlay/lib/python3.12/site-packages/setuptools_scm/git.py:163: UserWarning: "/home/runner/work/cpp-linter-hooks/cpp-linter-hooks" is shallow and may cause errors
        warnings.warn(f'"{wd.path}" is shallow and may cause errors')
      running bdist_wheel
      running build
      running build_py
      copying cpp_linter_hooks/util.py -> build/lib/cpp_linter_hooks
      copying cpp_linter_hooks/clang_tidy.py -> build/lib/cpp_linter_hooks
      copying cpp_linter_hooks/clang_format.py -> build/lib/cpp_linter_hooks
      copying cpp_linter_hooks/__init__.py -> build/lib/cpp_linter_hooks
      running egg_info
      writing cpp_linter_hooks.egg-info/PKG-INFO
      writing dependency_links to cpp_linter_hooks.egg-info/dependency_links.txt
      writing entry points to cpp_linter_hooks.egg-info/entry_points.txt
      writing requirements to cpp_linter_hooks.egg-info/requires.txt
      writing top-level names to cpp_linter_hooks.egg-info/top_level.txt
      adding license file 'LICENSE'
      writing manifest file 'cpp_linter_hooks.egg-info/SOURCES.txt'
      installing to build/bdist.linux-x86_64/wheel
      running install
      running install_lib
      running install_egg_info
      removing 'build/bdist.linux-x86_64/wheel/cpp_linter_hooks-0.0.post1.dev1-py3.12.egg-info' (and everything under it)
      error: [Errno 2] No such file or directory: 'build/bdist.linux-x86_64/wheel/cpp_linter_hooks-0.0.post1.dev1-py3.12.egg-info'
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building wheel for cpp_linter_hooks
Failed to build cpp_linter_hooks
ERROR: Could not build wheels for cpp_linter_hooks, which is required to install pyproject.toml-based projects

It looks like setuptools-scm doesn't like the pre-commit workspace (for some reason).

I would suggest you remove the unit testing from pre-commit-config.yml.

@shenxianpeng
Copy link
Collaborator Author

I tried to separate the pre-commit job into a single workflow to avoid running the pre-commit local hook and run pytest in the same environment, but no luck. CI passed in PR #36, but after merge CI failed again.

Finally, with your suggestion, I removed the local hook from pre-commit-config.yaml 59d68a1, and the pre-commit job is back to normal.

@2bndy5
Copy link
Contributor

2bndy5 commented Feb 27, 2024

The problem might have something to do with the reusable workflow's cache. Still it is better to have unit tests run separate from pre-commit hooks.

There's probably a reason that pytest doesn't have their own pre-commit hook. And the coverage tool automatically adds the working directory to python path. So, there shouldn't be any need to install cpp-linter-hook when using coverage run -m pytest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants