Skip to content

Modernize the extension and add a comprehensive test suite - #28

Merged
lkubb merged 6 commits into
mainfrom
tests-updates
Oct 12, 2026
Merged

lkubb merged 6 commits into
mainfrom
tests-updates

Conversation

@lkubb

@lkubb lkubb commented Oct 12, 2026 •

Copy link
Copy Markdown
Member

What does this PR do?

Modernizes the long-unmaintained extension and adds a comprehensive test suite as a foundation for managing the salt-extensions GitHub organization with it.

This is step 1 of my plan for this extension.
Step 2?: Refactor functions into separate per-entity modules (likely backwards-compatible, if at all).
Step 3: Introduce resource modules.
Step 4: Add more entities we need.

  • Adds a functional test suite (~220 tests, 99% coverage) backed by a stateful fake GitHub API server (tests/support/github/): a typed entity model served over HTTP, exercising the execution and state modules end-to-end (idempotence, drift, parametrized test mode, error paths). Deprecated API paths are intentionally unimplemented, so regressions to them fail loudly.
  • Reworks the execution module onto a single HTTP stack based on public PyGithub (>=2.2) APIs only — no more salt.utils.http, PyGithub internals (_requester, _rawData, ...) or deprecated endpoints (/teams/{team_id}/...).
  • Adds GitHub Enterprise support via a new api_url profile option.
  • Fixes many latent bugs the new tests exposed.
  • Tightens pylint configuration (including source-roots=src to fix bogus no-member errors caused by the saltext.github namespace shadowing PyGithub during static analysis).

Previous Behavior

  • Only a handful of mock-based unit tests existed (~11% coverage), so most behavior was unverified.
  • team_present with members always crashed (operator precedence bug).
  • ruleset_absent reported success when the ruleset could not be deleted.
  • ruleset_present diffed server-populated fields, causing perpetual drift, and test mode discarded forecast changes.
  • get_issues/get_prs/get_milestones/get_issue_comments crashed with AttributeError on API errors; is_team_member crashed on missing teams.
  • Several functions dropped the configured profile on internal cache refreshes; remove_repo raised instead of returning False for missing repos; repo_present failure comments were misleading.
  • Most states returned no changes in test mode.
  • The module relied on deprecated GitHub endpoints and private PyGithub APIs that could break on any PyGithub upgrade.

New Behavior

  • All mutating surfaces are covered by functional tests against the fake API; CI verifies behavior on all supported Salt/Python versions.
  • The listed bugs are fixed (see changelog entries for details).
  • All states report the same projected changes in test mode as they do when applied.
  • Only documented, non-deprecated GitHub API routes are used, exclusively through public PyGithub APIs.
  • Custom API URLs (GitHub Enterprise Server) are supported via the api_url profile option.

Merge requirements satisfied?

Commits signed with GPG?

Yes

@codecov

codecov Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.03175% with 105 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.06%. Comparing base (cd69ee4) to head (4fa1c18).

Files with missing lines Patch % Lines
tests/support/github/entities.py 91.71% 33 Missing and 14 partials ⚠️
tests/support/github/state.py 79.20% 28 Missing and 14 partials ⚠️
tests/support/github/routes/teams.py 93.39% 3 Missing and 4 partials ⚠️
tests/support/github/base.py 96.35% 2 Missing and 3 partials ⚠️
tests/support/github/routing.py 91.89% 2 Missing and 1 partial ⚠️
tests/support/github/routes/issues.py 97.22% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main      #28       +/-   ##
===========================================
+ Coverage   31.90%   97.06%   +65.15%     
===========================================
  Files           9       37       +28     
  Lines        1332     3573     +2241     
  Branches      233      335      +102     
===========================================
+ Hits          425     3468     +3043     
+ Misses        873       68      -805     
- Partials       34       37        +3     
Flag Coverage Δ
Linux 97.06% <96.03%> (+65.15%) ⬆️
macOS 97.06% <96.03%> (+65.15%) ⬆️
project 100.00% <100.00%> (+78.46%) ⬆️
py310 87.54% <83.18%> (+55.63%) ⬆️
py311 87.54% <83.18%> (+55.63%) ⬆️
py314 97.01% <95.94%> (+65.10%) ⬆️
salt_3006_27 87.54% <83.18%> (+55.63%) ⬆️
salt_3008_2 97.01% <95.94%> (+65.10%) ⬆️
tests 95.69% <95.47%> (-4.31%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The `no-member` errors are fixed by `source-roots` in pylint config, a
change which should make it into the template.

I tried renaming the execution module into `github_mod.py`, but that
results in breakage on <3008, which still contain the `github.py`
execution module. Its presence means no `__virtualname__` override
works.
@lkubb
lkubb merged commit 85ddda9 into main Oct 12, 2026
16 checks passed
@lkubb lkubb linked an issue Oct 13, 2026 that may be closed by this pull request
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.

[TECH DEBT] Need tests for existing modules

1 participant