Skip to content

refactor(agent-charm): move methods to charm utilities#935

Open
rene-oromtz wants to merge 2 commits intomainfrom
refactor/add-charm-utils
Open

refactor(agent-charm): move methods to charm utilities#935
rene-oromtz wants to merge 2 commits intomainfrom
refactor/add-charm-utils

Conversation

@rene-oromtz
Copy link
Contributor

Description

While trying to add charm code to the coverage report, noticed that it was very low on charm code side so before including coverage though on bumping up a little:

Current unreported coverage:

Name                                                            Stmts   Miss Branch BrPart  Cover
-------------------------------------------------------------------------------------------------
charms/testflinger-agent-host-charm/src/charm.py                  204    104     38      6    48%
charms/testflinger-agent-host-charm/src/common.py                  34      8      4      0    74%
charms/testflinger-agent-host-charm/src/config.py                  19      0      2      0   100%
charms/testflinger-agent-host-charm/src/defaults.py                 6      0      0      0   100%
charms/testflinger-agent-host-charm/src/supervisord.py             86     18     26      3    81%
charms/testflinger-agent-host-charm/src/testflinger_client.py      50      6      6      0    89%
charms/testflinger-agent-host-charm/src/testflinger_source.py      23      1      4      1    93%
-------------------------------------------------------------------------------------------------
TOTAL                                                             422    137     80     10    67%

Trying to keep it simpler for easier review so this PR moves some methods that can belong to a new module called "charm_utils", this covers all operation that belong to the charm lifecycle but that are not necessarily needed on charm code.

Also added a pydantic additional validation, instead of manually comparing config_dir and config_repo on each Charm Hook, I'm letting load_config() block the charm unit.

With the moved logic, coverage improved ~7%:

Name                                                            Stmts   Miss Branch BrPart  Cover
-------------------------------------------------------------------------------------------------
charms/testflinger-agent-host-charm/src/charm.py                  178     77     32      5    55%
charms/testflinger-agent-host-charm/src/charm_utils.py             30      0      4      0   100% <- New module
charms/testflinger-agent-host-charm/src/common.py                  34      8      4      0    74%
charms/testflinger-agent-host-charm/src/config.py                  25      0      4      0   100%
charms/testflinger-agent-host-charm/src/defaults.py                 6      0      0      0   100%
charms/testflinger-agent-host-charm/src/supervisord.py             86     18     26      3    81%
charms/testflinger-agent-host-charm/src/testflinger_client.py      50      6      6      0    89%
charms/testflinger-agent-host-charm/src/testflinger_source.py      23      1      4      1    93%
-------------------------------------------------------------------------------------------------
TOTAL                                                             432    110     80      9    74%

A follow up PR will add this coverage report

Resolved issues

N/A

Documentation

Web service API changes

Tests

Modify current tests to catch UncaughtCharmError triggered by Pydantic validation.
Also added unit tests for 100% coverage in charm_utils.py

@codecov
Copy link

codecov bot commented Feb 25, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.85%. Comparing base (9cbb26f) to head (4559108).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #935   +/-   ##
=======================================
  Coverage   73.85%   73.85%           
=======================================
  Files         108      108           
  Lines       10313    10313           
  Branches      886      886           
=======================================
  Hits         7617     7617           
  Misses       2508     2508           
  Partials      188      188           
Flag Coverage Δ *Carryforward flag
agent 74.40% <ø> (ø)
cli 89.56% <ø> (ø) Carriedforward from b2be3ff
device 59.86% <ø> (ø) Carriedforward from b2be3ff
server 87.85% <ø> (ø) Carriedforward from b2be3ff

*This pull request uses carry forward flags. Click here to find out more.

Components Coverage Δ
Agent 74.40% <ø> (ø)
CLI 89.56% <ø> (ø)
Common ∅ <ø> (∅)
Device Connectors 59.86% <ø> (ø)
Server 87.85% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@ajzobro ajzobro left a comment

Choose a reason for hiding this comment

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

Looks good, my few comments are mostly comment verbiage.

@rene-oromtz
Copy link
Contributor Author

@ajzobro Thanks for the feedback, I added the suggested modifications, in case anything else is needed please let me know!

@rene-oromtz rene-oromtz requested a review from ajzobro February 26, 2026 19:09
Copy link
Contributor

@ajzobro ajzobro left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for considering my changes!

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.

2 participants