Unit tests + coverage reports, type checking, pre-commit, and requested fixes #81

2bndy5 · 2022-08-09T04:42:47Z

Issues addressed

addresses lines-changed-only option does not work correctly #74
solves ignore option's CLI default value includes repo root #75
fixes submodules are only ignore if files_changed_only is True #76
resolves use setuptools_scm during install #77
partially-solves Why prepend a path to database? #73
- Using the database option in the docker env is currently discouraged (until we can switch to a composite action).
  - The docker container (via clang-tidy) seems to be following a symlink github/workspace that leads to a inaccessible absolute path. This is preventing any compilation database from being properly used in the docker env.
- The path provided to the database option is resolved into an absolute path (if it isn't already absolute). Thanks to @BurningEnlightenment for the suggestion!
- I changed the src to output any error from clang-tidy despite the executable's return code. This sheds more light on if clang-tidy was able to use the specified path to a compilation database.

And more

skip executing clang-format if the style option is set to a blank string ('')
allow any OS to accept a path to the installed tools via the version option. Previously, this was intended only for Windows OS, but it was needed to allow custom builds of the tools to be executed from a user-defined install path.
switched to rely more on pathlib instead of os for file system operations.
- with the switch to using pathlib.Path.rglob() (instead of using os.walk()), I was able to remove a slightly misleading debug statement as well.
moved install config to pyproject.toml, and updated the dockerfile accordingly. The setup.py file isn't actually used any more, but remains for older versions of pip.
added numerous unit tests and integrated codecov.io for coverage reporting.
simplified the stored list of files so that the same structure can be used for any supported event
improve translation from file's byte-offset to file's line & columns.
ensure the demo C++ sources use LF despite the OS that's checking out the repo. This was needed to get consistent results from the unit tests about the previous point.
added a CI workflow to publish to pypi or test-pypi. I also added the necessary pypi/test-pypi API tokens to this repo's secrets.
added type checking from mypy tool
added pre-commit config

- fix a few long line warnings - add rich to the py dev reqs - add types-PyYAML to dev reqs

normalize submodule path separators per runner's OS

also make sure the current working directory is appended to the .gitmodules path

- add pytest to dev reqs - rename run-pylint.yml to run-dev-tests.yml - install cpp-linter pkg in run-dev-tests.yml workflow - upgrade actions/setup-python to v2 in run-dev-tests.yml workflow - rename step in run-dev-tests.yml workflow

This allows the `lines-changed-only` option to accept the following values: - false: All lines in a file are analyzed - true: All lines in the diff are analyzed (including unchanged lines) - strict: Only lines in the diff that contain additions are analyzed gitignore some test resources as they are fetched running the tests. Retain original repo structure when fetching files. This is only used when the user doesn't checkout the repo being analyzed.

also fix a typo in demo.hpp variable name

2bndy5 · 2022-08-18T07:45:14Z

The file annotations seem to be in order also (see here). I'm much more confident now that we have 80% coverage and expected behavior in the unit tests.

shenxianpeng · 2022-08-18T07:52:21Z

Could you also please update README.md to introduce v2 in this PR or #83. I believe your introduction must be better than mine. Thank you.

2bndy5 · 2022-08-18T07:55:43Z

I think the v2 should go in #83 All the changes here were made with backward compatibility in mind. I do think there's some changes that should be made regarding the input options, so I'll re-read that file for accuracy.

shenxianpeng · 2022-08-18T07:58:15Z

Sure, please go ahead with #83

2bndy5 · 2022-08-18T08:03:24Z

Is the .ci-ignore file still needed?

cpp-linter-action/.ci-ignore

Lines 1 to 2 in a178730

    
           cpp_linter 
        
           mkdocs.yml

I recall this was for an external linter CI you had configured for the repo, but not sure if changing to an org account affected that.

shenxianpeng · 2022-08-18T08:11:21Z

I forgot it was configured by me. I even don't know what it used for, I think we don't need it

2bndy5 · 2022-08-18T08:34:11Z

I think I'll leave this alone until I'm sure the pkg is available on pypi

cpp-linter-action/README.md

Line 185 in a178730

run: python3 -m pip install git+https://github.com/cpp-linter/cpp-linter-action@v1

It would be nice to have all versions of the python source code up on pypi, but that's a lot of work for code that is known to be flawed.

2bndy5 · 2022-08-18T09:02:55Z

Please look over the readme changes. I also removed that .ci-ignore file.

2bndy5 · 2022-08-18T09:11:39Z

All good to go on my end. Here's hoping uploads to pypi don't have the same problems we experienced with cpp-linter/clang-tools-pip v0.3.0 release.

🤞🏼

I added the PyPI token to this repo...

shenxianpeng · 2022-08-18T09:23:19Z

Why not add PyPI token to cpp-linter repo, do you want to upload the package from this repo first and then migrate it to cpp-linter?

I used the same approach(in clang-tools-pip) and upload cpp-linter/cpp-linter-hooks to PyPI, everything goes well.

The only problem is you need to create a token (the scope is for all projects) for uploading if you don't have the project named cpp-linter in your pypi, then you have to create another token that scope is only for cpp-linter and update the token in the GitHub repo if you care about security.

2bndy5 · 2022-08-18T09:26:38Z

do you want to upload the package from this repo first and then migrate it to cpp-linter?

Yes. That would eliminate some guesswork from migration. PyPI doesn't care about the origin as long as the right credentials are used.

2bndy5 · 2022-08-18T09:30:37Z

According to the codecov action, the secret token isn't needed for public repos.

shenxianpeng · 2022-08-18T09:32:17Z

OK, please go ahead to remove the token.

2bndy5 · 2022-08-18T09:41:21Z

Done.

If you're using the upload script from codecov's instruction on the cpp-linter-hook repo, then switching to the codecov action will eliminate the need for that token in the org secrets.

I'm honestly not sure if the token is needed after uploading the first report.

shenxianpeng · 2022-08-18T10:19:19Z

I remember I copied the upload script and token from the codecov portal. It's Okay to remove the token and replaced with action, it should be easy to fix if any errors report.

2bndy5 · 2022-08-18T10:51:05Z

Done (with no problems).

shenxianpeng · 2022-08-18T11:08:38Z

Done (with no problems).

So ready to merge?

2bndy5 · 2022-08-18T11:10:12Z

yep

* add a submodule and update the CI checkout steps * Use NMake generator on Windows to get a DB * setup VS dev env in CI * don't use database option when running as action * do thread comments when run as action

2bndy5 added 30 commits July 26, 2022 15:26

integrate mypy type checking into CI

35c25d6

resolve #75

1374f8b

resolve #73

cf96533

run pylint workflow on changes to workflow file

4656598

- fix a few long line warnings - add rich to the py dev reqs - add types-PyYAML to dev reqs

resolve #76

fb5b09b

normalize submodule path separators per runner's OS

add unit tests for ignored paths

09034c3

also make sure the current working directory is appended to the .gitmodules path

run pytest in CI

cd60a49

- add pytest to dev reqs - rename run-pylint.yml to run-dev-tests.yml - install cpp-linter pkg in run-dev-tests.yml workflow - upgrade actions/setup-python to v2 in run-dev-tests.yml workflow - rename step in run-dev-tests.yml workflow

improve and test CLI arg parsing

a9d911a

add unit test for database

9e37e03

also fix a typo in demo.hpp variable name

switch to pre-commit; run pytest in a matrix

b5c721f

parametrize unit tests

4b4e18a

fix changes to dev-test yaml

b48ac79

make database path absolute

b0c3ebb

use pathlib to resolve() relative paths in test

8c89f45

let db path be independent of repo-root w/o docker

6fe57ef

this fails locally

c0b20bc

show me value for RUNNER_WORKSPACE

cf295d6

use gh workspace if not in docker env

a656f05

simplify the paths' concatenation

b2c751a

fix assignment of Globals.FILES

454e158

treat unsupport events like a push

6911a49

explicitly set a env var when using docker

767417a

ammend some logic

9518985

show me some contents of folders on docker

ccf8f33

hardcode the correct worspace dir

903d351

remove unused import

99bbfd9

try detecting the docker env better

8fa467e

set the docker env var w/ a str

400b615

assume that RUNNER_WORKSPACE is abs

0367cf4

2bndy5 marked this pull request as ready for review August 18, 2022 07:44

fix run-test.yml (trailing whitespaces)

a178730

shenxianpeng approved these changes Aug 18, 2022

View reviewed changes

revise README & remove .ci-ignore file

d3a8585

shenxianpeng self-requested a review August 18, 2022 09:07

shenxianpeng approved these changes Aug 18, 2022

View reviewed changes

add code coverage badge to README

b36ee44

shenxianpeng merged commit 600b1aa into master Aug 18, 2022

shenxianpeng deleted the backlogged-updates branch August 18, 2022 11:17

This was referenced Aug 19, 2022

the latest tag should be removed when the CI is done #84

Closed

Planned to release cpp-linter-action v2.0.0 (v2) #87

Closed

Unit tests + coverage reports, type checking, pre-commit, and requested fixes #81

Unit tests + coverage reports, type checking, pre-commit, and requested fixes #81

Uh oh!

Conversation

2bndy5 commented Aug 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issues addressed

And more

Uh oh!

2bndy5 commented Aug 18, 2022

Uh oh!

shenxianpeng commented Aug 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

2bndy5 commented Aug 18, 2022

Uh oh!

shenxianpeng commented Aug 18, 2022

Uh oh!

2bndy5 commented Aug 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shenxianpeng commented Aug 18, 2022

Uh oh!

2bndy5 commented Aug 18, 2022

Uh oh!

2bndy5 commented Aug 18, 2022

Uh oh!

2bndy5 commented Aug 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🤞🏼

Uh oh!

shenxianpeng commented Aug 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

2bndy5 commented Aug 18, 2022

Uh oh!

2bndy5 commented Aug 18, 2022

Uh oh!

shenxianpeng commented Aug 18, 2022

Uh oh!

2bndy5 commented Aug 18, 2022

Uh oh!

shenxianpeng commented Aug 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

2bndy5 commented Aug 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shenxianpeng commented Aug 18, 2022

Uh oh!

2bndy5 commented Aug 18, 2022

Uh oh!

Uh oh!

2bndy5 commented Aug 9, 2022 •

edited

Loading

shenxianpeng commented Aug 18, 2022 •

edited

Loading

2bndy5 commented Aug 18, 2022 •

edited

Loading

2bndy5 commented Aug 18, 2022 •

edited

Loading

shenxianpeng commented Aug 18, 2022 •

edited

Loading

shenxianpeng commented Aug 18, 2022 •

edited

Loading

2bndy5 commented Aug 18, 2022 •

edited

Loading