feat: DateTime params, Get-PiHoleConfig full tree, and CI reliability fixes - #95
Merged
Merged
Conversation
Adds Write-Host output (Format-List/Format-Table for structured results, direct interpolation for RawOutput) to every integration test so the actual data returned by the real server is visible in Pester's console output (with -Output Detailed), not just pass/fail. Also dropped two now-redundant Should -Not -Throw wrappers where the result is captured directly - an unhandled exception during that capture already fails the test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds Initialize-PiHoleTestData.ps1, which resolves a random mix of well-known domains and known ad/tracker domains directly against the Pi-hole's DNS resolver, so its query stats aren't all zero/empty. Wired into the BeforeAll of every stats-related integration test (live and database-backed). Live stats reflect the seeded queries immediately. The on-disk database stats only reflect them once FTL's periodic flush to the long-term database runs, so this mainly builds up real history across repeated runs rather than guaranteeing non-zero results within the same run - confirmed empirically before landing on this design. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
It only had a RawOutput branch and no else, so with RawOutput $false (the default) the function produced no output at all - not wrong data, no data. Rewrote it to match its sibling Get-PiHoleStatsTopClient.ps1: a proper try/catch/finally with session cleanup and a formatted Domain/Count object array in the default case. Verified against a real server - now returns real top-domain data instead of nothing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
test: print returned objects in integration tests
Get-PiHoleStatsDatabaseSummary, Get-PiHoleStatsDatabaseUpstream, Get-PiHoleStatsDatabaseTopDomain, Get-PiHoleStatsDatabaseTopClient, and Get-PiHoleStatsDatabaseQueryType previously required callers to pass raw Unix timestamps for -From/-Until. They now accept ordinary [datetime] values and convert internally via the existing (previously unused) Convert-LocalTimeToPiHoleUnixTime helper, so callers can pass things like (Get-Date).AddDays(-7) instead of computing epoch seconds by hand. Updated the corresponding integration tests to build DateTime values instead of Unix timestamps; verified all 5 functions and their tests against a real server. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…g to the last 8 hours Get-PiHoleStatsDatabaseSummary, Get-PiHoleStatsDatabaseUpstream, Get-PiHoleStatsDatabaseTopDomain, Get-PiHoleStatsDatabaseTopClient, and Get-PiHoleStatsDatabaseQueryType no longer require -From/-Until - they default to (Get-Date).AddHours(-8) and (Get-Date) respectively, matching the common case of "what's happened recently" without forcing every caller to specify a window. Added a test per function confirming the defaults work when both parameters are omitted; verified against a real server. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Get-PiHoleStatsSummary and Get-PiHoleStatsRecentBlocked declared [bool]$RawOutput with no explicit default - functionally identical to $false since that's bool's default value, but every other function in the module states it explicitly. Found while auditing all functions' RawOutput declarations for consistency. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
It previously defaulted to $null, but since the parameter is typed [int], PowerShell silently coerced that to 0 rather than actually omitting a timer value - contradicting the old docstring's claim that omitting it made the setting permanent. Making it mandatory forces callers to state their intent explicitly instead of relying on that silent coercion. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Get-PiHoleStatsQueryType and Get-PiHoleStatsDatabaseQueryType
previously returned a single wide object with A/AAAA/ANY/... as
top-level properties. They now return an array of {Type, Count}
objects instead, matching the row-based tabular pattern already used
by the Domain/Count and IP/Name/Count functions in this folder -
sortable and filterable with Sort-Object/Where-Object instead of
having to know all 16 property names up front.
While rewriting Get-PiHoleStatsQueryType, also fixed it to use the
module's standard try/catch/finally pattern with session cleanup,
which it was missing entirely, plus a stray reference to an
undefined $MaxResult variable in a Write-Verbose call.
Updated the corresponding integration test's assertions to match the
new array shape; verified both functions against a real server.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Every .EXAMPLE block used the same made-up gibberish string "fjdsjfldsjfkldjslafjskdl" for -Password. Replaced it with "your-app-password" everywhere for clarity. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds real-server integration tests for every exported Get-* function that didn't already have one: Get-PiHoleCurrentAuthSession, Get-PiHoleConfig, Get-PiHoleDnsBlockingStatus, Get-PiHoleInfoHost, Get-PiHoleInfoMessage, Get-PiHoleGroup, Get-PiHoleList, Get-PiHoleStatsQueryType (live), Get-PiHoleStatsRecentBlocked, Get-PiHoleStatsSummary, Get-PiHoleStatsTopClient, Get-PiHoleStatsTopDomain, and Get-PiHolePadd - 39 tests total, all passing against a real server. Get-PiHoleLogWebserver and Get-PiHoleTeleporterDownload are excluded: both exist as Public/ functions but aren't in the module's export list, so they can't actually be called after Import-Module. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Get-PiHolePadd was the only function left in the module with a
placeholder ".SYNOPSIS https://TODO" docstring. Replaced it with a
proper synopsis/description/parameter block matching the API spec's
actual summary ("Get summarized data for PADD") and added a HelpUri,
following the same convention as every other function.
Also fixed a real bug found while rewriting it: Cache.Evicted read
from $Reponse.cache.evicted (missing 's'), an undefined variable, so
it was always $null regardless of the actual value. Verified against
a real server that Evicted now returns the real value.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The module manifest's PSData.ReleaseNotes field was a static
placeholder ("Initial release targeting PowerShell 7+") that never
got updated, so the PowerShell Gallery package page always showed
that instead of anything about the actual release - unlike the
GitHub release, which auto-generates real "What's Changed" notes.
Generates the release notes once via the GitHub API
(releases/generate-notes) right after tagging, then uses that same
text for both the GitHub release body (via body_path, replacing
generate_release_notes: true) and the manifest's ReleaseNotes field
before Publish-Module runs, so both places show identical content.
The manifest update uses a surgical literal-text replace (matching
the existing lightweight ModuleVersion bump pattern already used in
this workflow) rather than Update-ModuleManifest, which rewrites and
reformats the entire file - verified locally that Update-ModuleManifest
drops comments, changes array literal styles, and silently comments
out VariablesToExport, none of which the targeted replace does.
Also removed a latent bug in the Publish step: it referenced
$ENV:TAG, which was never set (the actual env var is NEW_TAG), so
that redundant version-replace was already a silent no-op - the
"Zip folder" step's replace was the only one ever taking effect.
Verified locally: ran the full Zip-then-Publish manifest-editing
sequence against a scratch copy, including release notes containing
an apostrophe and a literal "$variable"-looking string, and confirmed
Test-ModuleManifest validates the result cleanly.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Get-PiHoleConfig's formatted output only ever captured Dns.Upstreams and the dhcp section - the API actually returns 9 top-level sections (dns, dhcp, ntp, resolver, database, webserver, files, misc, debug), each several levels deep, and everything outside those two hand-picked fields was silently discarded. Rather than hardcode this large, evolving tree by hand, added a generic recursive converter (ConvertTo-PiHolePascalCaseObject) that rebuilds any API response as nested PSCustomObjects/arrays with PascalCase property names - splitting on underscores and capitalizing each segment, matching the convention already used elsewhere in the module (EXTERNAL_BLOCKED_IP -> ExternalBlockedIp). Get-PiHoleConfig now returns the entire config faithfully instead of a lossy subset, and any new config keys Pi-hole adds in the future show up automatically instead of needing code changes. Also replaced its placeholder docstring (bare HelpUri, no parameters) with a proper one, and dropped the stray `break` in its catch block to match the module's standard error-handling pattern. Verified against a real server: all 9 top-level sections present, correct PascalCase for camelCase (ignoreLocalhost -> IgnoreLocalhost), snake_case (app_pwhash -> AppPwhash, client_history_global_max -> ClientHistoryGlobalMax), and acronym-heavy names (CNAMEdeepInspect, EDNS0ECS, blockESNI -> BlockESNI) left intact. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Remove-PiHoleCurrentAuthSession is best-effort logout cleanup called from every public function's finally block. Its catch block used Write-Error, which becomes a terminating exception under $ErrorActionPreference = 'Stop' - exactly what Azure Pipelines' pwsh task sets by default. A transient network hiccup while logging out (e.g. the server briefly unreachable right after a restart, which happened mid-run on the actual CI agent) was therefore failing whichever test happened to be running at the time, even though the function under test had already succeeded. Switched to Write-Warning, which never terminates regardless of $ErrorActionPreference. Reproduced the exact failure mode locally (Write-Error throws under -Stop, Write-Warning doesn't) before and after the fix to confirm. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
feat: accept local DateTime for From/Until on database stats functions
develop was made a protected branch requiring PRs for all changes, so this workflow's `git push origin HEAD:develop` step started failing every time it found real drift to fix (confirmed on PR #95's run: it correctly detected and committed the fix locally, then hit `GH006: Protected branch update failed... Changes must be made through a pull request` on push). Converted it to a check instead: runs the existing -Check mode and fails with instructions if the README is stale, the same way PSScriptAnalyzer already gates CI, rather than trying to auto-commit somewhere it no longer has permission to push. Also regenerated README.md itself, since the failed auto-push meant develop's copy never actually got the update - it now reflects the docstring fixes from PR #94 (Get-PiHoleConfig, Get-PiHolePadd, Get-PiHoleStatsQueryType, Get-PiHoleStatsTopDomain). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fix: README sync workflow can't push to a now-protected develop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Get-PiHoleStatsDatabaseSummary,Get-PiHoleStatsDatabaseUpstream,Get-PiHoleStatsDatabaseTopDomain,Get-PiHoleStatsDatabaseTopClient, andGet-PiHoleStatsDatabaseQueryTypenow accept ordinary[datetime]values for-From/-Until(optional, defaulting to the last 8 hours) instead of requiring raw Unix timestamps.Also included
$falsedefaultSet-PiHoleDnsBlocking'sTimeInSecondsis now a required parameterGet-PiHoleStatsQueryType,Get-PiHoleStatsDatabaseQueryType) to return{Type, Count}row arrays instead of one wide object.EXAMPLEblockGet-*functionGet-PiHolePadd: replaced its placeholderTODOdocstring and fixed a typo bug (Cache.Evictedalways$null)Get-PiHoleConfignow returns the full config tree (all 9 top-level sections instead of 2), via a new generic recursive PascalCase converter, rather than a hand-picked lossy subsetRemove-PiHoleCurrentAuthSession, best-effort logout in every function'sfinallyblock) were failing unrelated tests on the real Azure DevOps agent, becauseWrite-Errorbecomes terminating under$ErrorActionPreference = 'Stop'(Azure Pipelines'pwshtask default). Switched toWrite-Warning.Get-PiHoleStatsTopDomainsilently returning nothing by default.Test plan
Invoke-Pester -Path .\testsrun — 77 passed, 0 failedInvoke-ScriptAnalyzer -Path .\PiHoleShell -Recurse— no findingsGet-PiHoleConfigand the database-statsDateTimeparams against a real serverPesterPiHoleShellcheck passes (reproduced and fixed the root cause after finding it in the actual CI log)🤖 Generated with Claude Code