feat: add automated test workflow for certificate validation#11
feat: add automated test workflow for certificate validation#11oto-macenauer-absa wants to merge 7 commits into
Conversation
oto-macenauer-absa
requested review from
miroslavpojer and
ndivho-makhuvha
as code owners
There was a problem hiding this comment.
Pull request overview
Adds a CI workflow to exercise the composite validate-certificates action across multiple certificate validity scenarios on both Linux and macOS runners.
Changes:
- Introduces
.github/workflows/test.ymlto generate certificate fixtures at runtime and validate action behavior across 6 scenarios. - Updates
.gitignoreto exclude generated test fixtures (tests/fixtures/) from local working copies.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
.github/workflows/test.yml |
New cross-platform workflow that generates cert fixtures and asserts expected outcomes/step-summary content for key scenarios. |
.gitignore |
Ignores runtime-generated test fixture directory to keep the repo clean for local runs. |
| steps: | ||
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 | ||
| with: | ||
| persist-credentials: false |
There was a problem hiding this comment.
Even the referenced code clearly says actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd, nothing to fix here
oto-macenauer-absa
force-pushed
the
feat/add-test-workflow
branch
from
b529503 to
a93781c
Compare
oto-macenauer-absa
force-pushed
the
feat/add-test-workflow
branch
from
a93781c to
e05b006
Compare
The refactor that extracted inline bash to validate.sh dropped fail_on_warn support, output variable emissions, and the metrics table from the step summary. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| echo "Fixtures generated:" | ||
| ls -la "$FIXTURES_DIR"/*.pem | ||
|
|
||
| - name: 'Test: valid certificate' |
There was a problem hiding this comment.
This looks awesome overrall, one thing i spotted is that the case for fail_on_warn test case is not added, another one is that we do not test if given certs are valid JSON.
There was a problem hiding this comment.
I've added those test cases, it should be complete now
Addresses review feedback on PR #11: the fail_on_warn flag and JSON input validation were not exercised by the test workflow. - fail_on_warn=true fails on an expiring-soon cert (exit 1) - fail_on_warn=true still passes a valid cert (exit 0) - malformed JSON input is rejected (exit 1) - valid JSON that is not an array is rejected (exit 1) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ndivho-makhuvha
left a comment
ndivho-makhuvha
left a comment
There was a problem hiding this comment.
Looks great, approved
|
Sorry, before merging the test of fail_on_warn but does not fail is not giving a proper summary, on status it showed its a valid cert, instead of expiring soon. since fail on warn, validates certificates nearing expiry, the status should also reflect that. |
Review feedback on PR #11: the prior "does not fail" test used a plain valid cert, so the summary showed "Valid" rather than the nearing-expiry status the scenario is meant to demonstrate. Use an expiring cert that shares a subject with a newer valid replacement. With fail_on_warn=true this must not fail (WARN_COUNT stays 0), and the summary now correctly shows "Expiring Soon (newer replacement exists)". Adds the multi_expiring.pem fixture for this case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Good catch, fixed in 38d8964. The "does not fail" test previously used a plain valid cert, so the summary showed "Valid" rather than reflecting the nearing-expiry state. It now uses an expiring-soon cert that shares a subject with a newer valid replacement (new |
Summary
.github/workflows/test.ymlwith 6 test scenarios covering valid, expiring-soon, expired, not-yet-valid, missing file, and multi-cert with valid replacementopenssl reqandopenssl cafor date-controlled certsubuntu-latestandmacos-latestto catchdateportability regressionsCloses #6
Release Notes
Test plan