283 lines
27 KiB
Markdown
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.
|