Skip to content

addons/namingng.py: Documentation and standalone unit test.#5841

Closed
mvds00 wants to merge 7 commits into
cppcheck-opensource:mainfrom
Rail-Connected:pullreq11
Closed

addons/namingng.py: Documentation and standalone unit test.#5841
mvds00 wants to merge 7 commits into
cppcheck-opensource:mainfrom
Rail-Connected:pullreq11

Conversation

@mvds00
Copy link
Copy Markdown
Contributor

@mvds00 mvds00 commented Jan 5, 2024

As promised:

  • addons/README.md documenting namingng.py
  • releasenotes.txt describing recent changes to namingng.py
  • expanded namingng.py unit test to cover standalone mode (performing the exact same test)

I decided, for now, to skip the request for an additional unit test for namingng config without function/variable prefixes. All configuration options of namingng.py are optional, so it feels arbitrary to add a test only for the absence of this particular setting. If we can come up with a clear strategy for such testing and decide it adds tangible value, then I'd be happy to implement it - let's discuss.

Comment thread addons/README.md Outdated

![Screenshot](https://raw.githubusercontent.com/danmar/cppcheck/main/addons/doc/img/cppcheck-gui-addons.png)

### Using namingng.py
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just feedback on the struct and not he contents.

I guess this could be restructured into a "description", "configuration" and "usage" sub-section.

The invocation should be moved into a separate generic section as it applies to all addons.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, will change.

Comment thread test/cli/test-other.py Outdated
expect.sort()
assert lines == expect

# Perform same analysis again, but now in standalone mode.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably move all the standalone addon tests from being a non-portable, inline command-line script to Python.

I also think we should move the addon tests to a separate script. That is probably true for several things in test-other.py which is basically just a dumping ground for most non-project tests.

Both is nothing for this PR but for future cleanups.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both is nothing for this PR but for future cleanups.

Fine with me to do this while I'm at it...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I force pushed a split of test-other.py into test-addons.py on this PR.

mvds00 added 5 commits January 5, 2024 20:53
With a little help from my artificially intelligent friend.

Note that literal _ needs escaping in markdown format.
This reuses the existing unit test, running the addon on the dump file created.

The format of the standalone mode is different so the expected output is
slightly transformed before checking.

This implicitly also tests `cppcheck --dump`, i.e. whether checking 1 file
leads to 1 dump file, and whether the dump filename is indeed file+'.dump'.
This is relevant as some code simply assumes this to be the case.
@firewave firewave marked this pull request as draft January 5, 2024 21:55
@mvds00
Copy link
Copy Markdown
Contributor Author

mvds00 commented Jan 8, 2024

This PR will become part of #5852

@mvds00 mvds00 closed this Jan 8, 2024
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 this pull request may close these issues.

2 participants