Files
ansible-workspace-dnd/docs/roadmap/test-suite-audit-findings.md

283 lines
27 KiB
Markdown

# Test Suite Audit Findings
Status: resolved
## Executive Summary
All three integration suites pass, and each includes the most important common
behavioral check: an immediate repeated role application reports no changes.
The suites also contain useful coverage of explicitly selected releases,
caller-supplied Go executables, missing-binary recovery, configuration
rendering, native configuration validation, and Notarius pipeline composition.
The suites are nevertheless substantially larger and more implementation-bound
than the workspace testing policy permits. The recurring problems are:
- exact assertions about the current default release and its commit;
- independent verification of ownership, permissions, checkout state, and
template output already managed by trusted Ansible modules;
- parsing Ansible output for specific task names and individual
`changed`/`skipped` results;
- deliberate metadata drift followed by tests of `ansible.builtin.file`;
- three-case negative tests of self-evident platform assertions;
- repeated clean-run assertions after nearly every scenario; and
- duplicated final shell assertions after equivalent Ansible verifiers.
The appropriate remediation is subtraction and consolidation. The retained
suites should exercise stable role-owned behavior through final application
outcomes, with one representative case for each meaningful behavior.
## Baseline
The audit ran on Ubuntu 26.04 `x86_64` with Ansible Core 2.21.3 and
ansible-lint 26.8.0. The role revisions were:
| Repository | Revision | Integration result |
| --- | --- | --- |
| `ansible-role-seriatim` | `fb8dc41` | Pass |
| `ansible-role-audita` | `847c4c3` | Pass |
| `ansible-role-notarius` | `eb4f511` | Pass |
The following commands all exited successfully:
```text
(cd ansible-role-seriatim && tests/integration)
(cd ansible-role-audita && tests/integration)
(cd ansible-role-notarius && tests/integration)
```
Each driver installed its pinned collection requirements into a private
temporary directory, ran a syntax check and production-profile ansible-lint,
executed its behavioral scenarios, and restored its default deployment during
cleanup. The runs emitted non-fatal local-tooling warnings about Python
interpreter discovery or an already installed duplicate collection; these did
not affect the results or any classification below.
All three role repositories were clean before and after the audit. The control
repository began dirty solely because the new testing convention, audit plan,
and status update were awaiting commit.
## Test Inventory
| Repository | Entry point and supporting files |
| --- | --- |
| `ansible-role-seriatim` | `tests/integration`, `tests/inventory`, `tests/converge.yml`, `tests/verify.yml`, `tests/unsupported.yml`, `collections/requirements.yml`, and the README validation section |
| `ansible-role-audita` | `tests/integration`, `tests/inventory`, `tests/converge.yml`, `tests/verify.yml`, `tests/verify-config.yml`, `tests/unsupported.yml`, `collections/requirements.yml`, and the README validation section |
| `ansible-role-notarius` | `tests/integration`, `tests/inventory`, `tests/converge.yml`, `tests/verify.yml`, `tests/verify-config.yml`, `tests/unsupported.yml`, `collections/requirements.yml`, and the README validation section |
## Cross-Repository Findings
| Repository | Test or case | Behavior protected | Classification | Recommended action | Rationale |
| --- | --- | --- | --- | --- | --- |
| All three | Pinned collection installation, syntax check, and ansible-lint | Static validity and reproducible dependencies | Retain | Keep as static validation before behavioral scenarios. | These checks complement rather than duplicate behavioral testing. |
| All three | `require_clean_recap` after an immediate repeat | Role-level repeated convergence | Retain | Keep at least one immediate clean repeat per role and repeats specifically required after missing-binary recovery. | Overall `changed=0` is a first-class public role outcome. |
| All three | `require_changed_recap` after each mutating setup | Ansible reports some change on the first run | Remove | Stop asserting a nonzero aggregate change count except where a future requirement specifically makes reporting itself significant. | Final application state is the behavior of interest; these checks add little beyond successful convergence and make scenario sequencing brittle. |
| All three | Greps for named build, metadata, and configuration tasks | Current task names and individual `changed` or `skipped` results | Remove | Delete all task-output parsing. | These are direct implementation traces and fail under behavior-preserving task refactors. |
| All three | Binary/source/configuration ownership and mode assertions | Postconditions declared to `file`, `git`, and `template` | Remove | Delete independent metadata and recursive-ownership verification. | These tests re-prove trusted module behavior rather than role-authored decisions. |
| All three | Binary mode mutation followed by convergence | `ansible.builtin.file` repairs mode drift without building | Remove | Delete the scenario. | It is a focused regression test for the built-in file module and current task arrangement. |
| All three | Debian, old Ubuntu, and unsupported-architecture matrix | Straightforward platform assertion branches | Remove | Delete `tests/unsupported.yml` and the three-case driver loop. | The declarative assertion is self-evident and does not warrant a negative matrix. |
| All three | Restoration of the current default followed by verification and another repeat | Test cleanup and the current default identity | Simplify | Keep one best-effort cleanup convergence, but do not treat it as another test or assert the restored release value. | Cleanup is useful; testing a mutable default and repeating already established idempotence are not. |
| All three | Final shell assertions duplicating earlier verifiers | Current commit, metadata, version, or config state | Remove | Delete duplicate end-of-driver assertions. | They add maintenance cost without protecting a distinct behavior. |
## Seriatim Classification
| Repository | Test or case | Behavior protected | Classification | Recommended action | Rationale |
| --- | --- | --- | --- | --- | --- |
| Seriatim | Default convergence | The role can be applied with its default interface | Retain | Keep as a smoke test without asserting the exact default tag, commit, or filesystem metadata. | A default invocation is useful even though the present default value is not a stable test expectation. |
| Seriatim | Default `v1.5.0` and commit verification | Present default release identity | Remove | Remove the fixed `default_commit` and exact default-version assertions. | This is a change detector; advancing the supported default should not break unrelated tests. |
| Seriatim | Baseline immediate repeat | Clean convergence | Retain | Keep the overall `changed=0`, `failed=0` check. | This directly expresses the agreed idempotence requirement. |
| Seriatim | Missing binary | The imperative build runs when its output is absent and then converges | Simplify | Delete the binary, apply the role, verify the executable through `--version`, and require the next run to be clean; remove task-name greps. | The binary-existence trigger is role-authored behavior, while the current task name and status are not. |
| Seriatim | Dependency opt-out and custom Go executable | The caller-supplied build executable is honored | Simplify | Retain the wrapper invocation evidence, preferably combined with the missing-binary scenario; remove build-task greps and fixed-default assertions. | The selected build executable is a public role choice that cannot be observed reliably from the final binary alone. |
| Seriatim | Binary metadata repair | Built-in file convergence | Remove | Delete the complete scenario. | It tests `ansible.builtin.file`, not Seriatim-specific behavior. |
| Seriatim | Explicit `v1.4.0` selection | A caller-selected immutable release is installed | Simplify | Retain explicit `seriatim_version: v1.4.0` and verify `seriatim version v1.4.0`; remove the expected commit and task-state assertions. | This is the stable version-selection test described by the testing convention. |
| Seriatim | Restore `v1.5.0` default | Cleanup plus present default identity | Simplify | Reduce to cleanup only. | Exact current-default verification duplicates the harmful baseline snapshot. |
| Seriatim | Platform failure matrix | Distribution, release, and architecture assertions | Remove | Delete the matrix and `tests/unsupported.yml`. | No plausible role regression justifies separately testing each simple assertion. |
| Seriatim | `tests/verify.yml` installation contract | Commit, source metadata, binary metadata, and version | Replace | Replace the broad verifier with a narrow native version check used for an explicitly selected tag. | Only the application outcome is valuable; the remaining assertions repeat trusted modules. |
| Seriatim | README validation claims | Description of the current large matrix | Replace | Describe the lean retained behaviors after remediation. | Documentation should not promise deleted implementation-trace and module tests. |
## Audita Classification
| Repository | Test or case | Behavior protected | Classification | Recommended action | Rationale |
| --- | --- | --- | --- | --- | --- |
| Audita | Default convergence | The role installs and configures successfully through its default interface | Retain | Keep as a smoke test and validate the resulting configuration with Audita; do not inspect exact default identity or metadata. | Successful native validation is an application outcome and does not freeze the default document. |
| Audita | Default commit assertion | Present default checkout identity | Remove | Delete `default_commit` and exact commit verification. | It is both a mutable-default snapshot and a re-test of `ansible.builtin.git`. |
| Audita | Source, binary, and configuration metadata | Trusted module postconditions | Remove | Delete the ownership, permission, regular-file, and executable assertions. | The role directly declares these states with trusted modules. |
| Audita | Exact minimal default document `{"version": 1}` | Current default rendering | Remove | Do not snapshot the complete default document. Test null omission with explicitly supplied inputs in a representative configuration case instead. | A future intentional concrete default would otherwise cause an unrelated failure. |
| Audita | Native validation of the managed configuration | The role produces configuration Audita accepts | Retain | Keep one native validation for default smoke coverage and one for the representative explicit configuration. | This observes application behavior rather than reconstructing its schema. |
| Audita | Baseline immediate repeat | Clean convergence | Retain | Keep one role-level clean repeat. | This is required public behavior. |
| Audita | Missing binary | Absent build output is recreated and convergence resumes | Simplify | Retain recovery and the following clean repeat; prove usability with the Audita CLI and remove task-output greps. | The imperative build trigger is role logic, but its task name is not. |
| Audita | Dependency opt-out and custom Go executable | Caller-selected compiler path is used | Simplify | Retain wrapper invocation evidence and combine it with missing-binary recovery; remove task greps and redundant repeat runs. | This covers a documented public choice with one representative case. |
| Audita | Binary metadata repair | Built-in file convergence | Remove | Delete the complete scenario. | It tests Ansible's file module. |
| Audita | Representative configuration containing nearly every field | Field-to-YAML mapping, nested sections, and native validity | Simplify | Use a smaller explicit sample spanning representative scalar, list, nested, and duration shapes. Assert only selected paths and specifically selected omissions, then run native validation. | Full-document equality across nearly every field unnecessarily ossifies the template and public interface. |
| Audita | Explicit falsey configuration | Empty lists and strings are rendered rather than omitted | Retain | Keep representative falsey values, preferably in the same explicit configuration scenario, and assert only the affected paths. | The null-versus-falsey distinction is role-authored and easy to regress. |
| Audita | Configuration task changed while build task skipped | Configuration changes independently of installation | Remove | Delete task-name and status inspection. Confirm the rendered application behavior and successful convergence instead. | Whether named tasks ran is an implementation detail; the ordinary `when` expressions are suitable for review and lint. |
| Audita | Restore the minimal default configuration | Cleanup and exact current default | Simplify | Keep restoration only as cleanup and remove fixed-document, fixed-commit, task-state, and extra repeat assertions. | It does not add a distinct stable behavior. |
| Audita | Alternate raw commit `46b7356...` | A non-default Git revision can be selected | Replace | Remove the raw-commit case. Add an explicit immutable-release test only when Audita exposes a stable native way to identify the installed application release. | A raw source commit checked with `git rev-parse` tests checkout mechanics, not the installed application's release. Audita currently has no `--version` or `version` command, and tags `v0.9.0` and `v0.9.1` both resolve to `a84941d...`, so neither tag provides a distinguishable installed outcome today. |
| Audita | Platform failure matrix | Straightforward platform assertions | Remove | Delete the matrix and `tests/unsupported.yml`. | This is expressly outside the testing house style. |
| Audita | Final commit, metadata, and config checks | Duplicate checkout/module/validation assertions | Remove | Delete them. | Equivalent checks already ran, and most should themselves be removed. |
| Audita | `tests/verify.yml` broad installation contract | Commit, metadata, default document, and native config validity | Replace | Reduce it to native configuration validation; move explicit field assertions to the focused representative verifier. | Only native validity is a stable application outcome in this verifier. |
| Audita | README validation claims | Description of default, metadata, rebuild, and platform tests | Replace | Describe only the retained static checks and behavioral scenarios. | The present prose promises low-value coverage that should be removed. |
### Audita Version-Coverage Gap
The current suite does not—and with the present application interface cannot—
perform the useful test “select immutable release X and verify that the
installed Audita reports X.” Running either `audita --version` or
`audita version` fails as an unknown command. The recommended role-test
remediation is to remove the misleading raw-commit scenario rather than
replace it with another Git-module assertion. A native Audita release-identity
command would enable the desired explicit-version test later, but adding that
application feature is outside this audit and should be selected separately.
## Notarius Classification
| Repository | Test or case | Behavior protected | Classification | Recommended action | Rationale |
| --- | --- | --- | --- | --- | --- |
| Notarius | Default convergence | The default public interface installs and configures successfully | Retain | Keep as a smoke test, run `notarius --version` without fixing its output to the current default, and validate the configuration natively. | This proves the default path remains usable without snapshotting its values. |
| Notarius | Default `v0.6.0` and commit verification | Present default release identity | Remove | Delete fixed `default_commit` and exact default-version assertions. | These fail solely when the supported default advances. |
| Notarius | Source, binary, and configuration metadata | Trusted module postconditions | Remove | Delete ownership, mode, and regular-file assertions. | They re-test `git`, `file`, and `template`. |
| Notarius | Exact minimal default document `{"version": 4}` | Current default rendering | Remove | Do not snapshot the default document. Supply `notarius_config_version` explicitly in the representative case and make any omission assertions about explicitly null inputs. | This preserves schema-bound behavior without turning future defaults into failures. |
| Notarius | Native version command and configuration validation | Installed CLI is usable and accepts the managed configuration | Retain | Keep both native interfaces, but assert an exact version only in the explicitly selected release scenario. | These are application-level outcomes. |
| Notarius | Baseline immediate repeat | Clean convergence | Retain | Keep one overall clean repeat. | This directly enforces the idempotence requirement. |
| Notarius | Missing binary | The required output is rebuilt and then converges | Simplify | Retain artifact removal, native CLI verification, and the following clean repeat; remove task greps. | Missing-output recovery is role logic needed around the imperative build. |
| Notarius | Dependency opt-out and custom Go executable | The caller-selected build executable is honored | Simplify | Keep wrapper invocation evidence and combine with missing-binary recovery; remove task greps and redundant repetitions. | This is meaningful configurable behavior with no better final-state discriminator. |
| Notarius | Binary metadata repair | Built-in file convergence | Remove | Delete the complete scenario. | It tests trusted module behavior. |
| Notarius | Explicit `v0.5.0` selection | A caller-selected immutable tag becomes the installed application | Simplify | Retain `notarius_version: v0.5.0` and exact native output `notarius v0.5.0`; remove expected commit and task-state assertions. | This remains useful after the role's default changes. |
| Notarius | Restore `v0.6.0` default | Cleanup and present default identity | Simplify | Reduce to cleanup only. | It is not a distinct stable behavioral test. |
| Notarius | Representative configuration | Field mapping, nested sections, pipelines, and validity | Simplify | Use a smaller set of explicit values and assert only those paths plus specifically selected null omissions; retain native validation. | Exact equality for a large complete document is broader and more brittle than the behavior requires. |
| Notarius | Separate `profile_dir` case | One simple optional leaf mapping | Remove | Do not maintain a standalone scenario; a future representative case may select one profile source if it adds value. | The direct conditional and serialization are self-evident and already represented by other optional fields. |
| Notarius | Separate `profile_file` case | A second simple optional leaf mapping | Remove | Delete the standalone scenario. | It duplicates the shape and risk of the profile-directory case without protecting a distinct likely regression. |
| Notarius | Explicit falsey values | Zero, false, empty mapping, and empty string survive rendering | Retain | Fold representative falsey values into the focused configuration scenario and assert only those paths. | Null-versus-falsey behavior is a deliberate role invariant. |
| Notarius | Caller-side pipeline composition | Independently defined mappings can be combined and passed through intact | Retain | Keep composition through ordinary `combine`, assert the two resulting pipeline entries, and run native validation; fold it into the representative configuration scenario if clarity permits. | This is an expressly selected public boundary and a plausible integration regression. |
| Notarius | Configuration task changed while build task skipped | Configuration changes without rebuilding | Remove | Delete `require_configuration_change_without_build` and all task-trace greps. Verify final configuration, CLI usability, and clean repeat instead. | Task execution is not a stable public interface. |
| Notarius | Restore the minimal default configuration | Cleanup plus mutable default assertions | Simplify | Keep cleanup only; remove exact default document, version, commit, task-state, and extra repeat checks. | The behavioral assertions duplicate change detectors elsewhere. |
| Notarius | Platform failure matrix | Straightforward platform assertions | Remove | Delete the matrix and `tests/unsupported.yml`. | It provides no proportionate role-specific value. |
| Notarius | Final commit, metadata, version, and config checks | Duplicate assertions | Remove | Delete them. | They repeat earlier verification and freeze the current default. |
| Notarius | `tests/verify.yml` broad installation contract | Commit, metadata, default document, version, and validity | Replace | Reduce to native CLI smoke and configuration validation, with exact version verification only for an explicit tag. | This retains application outcomes while removing module tests and mutable snapshots. |
| Notarius | README validation claims | Description of the current broad suite | Replace | Rewrite after simplification to describe the retained behavior. | Documentation should match the lean suite rather than preserve obsolete coverage promises. |
## Proposed Lean Target Suites
### Seriatim
1. Install pinned collections, syntax-check the convergence play, and lint the
role and retained test files.
2. Apply the role with defaults as a smoke test and immediately require a clean
repeat, without asserting the current default tag or commit.
3. Select `v1.4.0` explicitly and verify native output
`seriatim version v1.4.0`.
4. Delete the binary, supply a wrapper through `seriatim_go_binary` with
dependency management disabled, converge, verify that the wrapper was used
and the installed CLI works, then require a clean repeat.
5. Restore defaults as cleanup without treating the restored value as a test
expectation.
### Audita
1. Install pinned collections, syntax-check the convergence play, and lint the
role and retained test files.
2. Apply the role with defaults, validate the managed configuration with
Audita, and immediately require a clean repeat. Do not assert the complete
default document or current source identity.
3. Apply one representative explicit configuration spanning nested, list,
scalar, duration, falsey, and explicit-null behavior. Assert only the
selected paths and omissions, validate it with Audita, and require a clean
repeat.
4. Delete the binary, supply a logging wrapper through `audita_go_binary` with
dependency management disabled, converge, verify wrapper use and CLI
usability, then require a clean repeat.
5. Omit version-selection coverage until Audita exposes a native release
identity; do not substitute a raw-commit checkout test.
6. Restore defaults as cleanup only.
### Notarius
1. Install pinned collections, syntax-check the convergence play, and lint the
role and retained test files.
2. Apply the role with defaults, exercise `notarius --version`, validate the
managed configuration, and immediately require a clean repeat without
asserting the current default values.
3. Select `v0.5.0` explicitly and verify native output `notarius v0.5.0`.
4. Delete the binary, supply a logging wrapper through `notarius_go_binary`
with dependency management disabled, converge, verify wrapper use and CLI
usability, then require a clean repeat.
5. Apply one representative explicit configuration that includes selected
nested values, explicit falsey and null values, and two caller-composed
pipeline mappings. Assert only those behaviors, validate the file with
Notarius, and require a clean repeat.
6. Restore defaults as cleanup only.
## Documentation Follow-up
The README validation sections in all three role repositories should be
rewritten with their test harnesses. They currently advertise current-default,
metadata, task-arrangement, and platform-guard coverage recommended for
removal.
The durable feature roadmaps may continue to record the selected default tags,
paths, ownership, permissions, and platform support as feature requirements.
Those requirements need not each have a dedicated integration assertion.
Test-specific roadmap language should, however, be corrected as follows:
- Feature 0001 and Feature 0003 should describe exact native version checks as
checks of an explicitly selected immutable tag, not the current default.
- Feature 0002 currently says repository-local tests execute Audita to verify
its installed version. That is not implementable with Audita's current CLI
and should be replaced with the limitation and follow-up described above.
- Platform support remains part of each role contract, but the roadmaps should
not imply that each straightforward assertion requires a negative matrix.
## Recommended Remediation Order
1. Simplify the Seriatim driver and verifier, remove its unsupported-platform
playbook, and update its README.
2. Simplify Audita's driver and verifiers, consolidate its configuration case,
remove its unsupported-platform playbook, and update its README.
3. Simplify Notarius's driver and verifiers, consolidate configuration and
pipeline coverage, remove its unsupported-platform playbook, and update its
README.
4. Correct the test-specific language in the three feature roadmaps.
5. Run all three complete integration entry points and confirm every role
repository remains clean after its cleanup path.
No shared test abstraction is recommended. The three shell drivers have
similar mechanics, but keeping each suite local and readable is worth the
small amount of duplication, and a shared helper would create cross-repository
coupling for little present benefit.
## Audit Completion
Every materially distinct implemented scenario has been classified above. The
recommended removals either duplicate retained application-level coverage or
exercise behavior owned by trusted Ansible modules or straightforward
assertions. The lean targets retain default smoke coverage, immediate clean
convergence, imperative missing-output recovery, caller-supplied build tooling,
explicit release selection where natively observable, and representative
configuration behavior.
The audit made no changes to production roles or test implementations.
## Remediation Status
The recommended remediation was implemented and independently reviewed. No
production role behavior changed. The role commits are:
| Repository | Remediation commit | Final integration result |
| --- | --- | --- |
| `ansible-role-seriatim` | `e08c63c` | Pass |
| `ansible-role-audita` | `d2d0b9d` | Pass |
| `ansible-role-notarius` | `cfecf37` | Pass |
Final sequential validation on the approved Ubuntu 26.04 `x86_64` proxy ran
each repository's `tests/integration` entry point successfully. Each cleanup
path restored the default deployment, and the role repositories were clean
after their remediation commits. The existing classifications remain the
authoritative rationale for this implemented suite shape.
Independent review found no remaining code-quality issues or implementation
gaps. The three complete integration entry points passed again sequentially;
Bash syntax validation and `git diff --check` passed; native application checks
confirmed that cleanup restored usable default deployments; and every
repository in the workspace remained clean. No additional remediation record
was required.