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

(WIP) Test missing logging call linter #764

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

marceloneppel
Copy link
Member

@marceloneppel marceloneppel commented Nov 11, 2024

Issue

We usually forget to add logging calls at the early exits of the charm code.

Solution

Add a linter to catch missing logging calls and report them as a warning (maybe block a PR from being merged in the future)

How to test this PR:

# Set up Vex. You need Rust for it: curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh
git clone https://github.com/TheSignPainter98/vex
cd vex/
cargo install --path .

# Check out this PR.
cd ..
git clone https://github.com/canonical/postgresql-k8s-operator
cd postgresql-k8s-operator
git checkout test-missing-logging-call-linter

# Check for the missing logging calls.
vex check

Sample output:

warning[logging]: return statement without logging call
   --> src/relations/async_replication.py:526:9
    |
526 |           if self._wait_for_standby_leader(event):
    |  _________-
527 | |             return
    | |__________________- consider adding the logging call
    |
warning: found 1 problem

TODO:

  • Check the elif and else clauses.
  • Ignore cases where a call to another function already did some logging call on that function.
  • Add vex check to the CI workflow.

Signed-off-by: Marcelo Henrique Neppel <[email protected]>
Copy link

codecov bot commented Nov 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.56%. Comparing base (293b597) to head (c4c869f).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #764      +/-   ##
==========================================
- Coverage   75.65%   75.56%   -0.10%     
==========================================
  Files          12       12              
  Lines        3110     3110              
  Branches      474      474              
==========================================
- Hits         2353     2350       -3     
- Misses        614      617       +3     
  Partials      143      143              

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

Signed-off-by: Marcelo Henrique Neppel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant