airvzxf / airvzxf/ftp-deployment-action

test(integration): no end-to-end scenario covers INPUT_DRY_RUN=true against a real FTP server

Open
#323 0 comments 0 reactions 0 assignees View on GitHub
area:tests audit pending-design test
Dominant language
Shell
Stars
37
Forks
9
Avg merge
44m
Merged PRs (30d)
47

Description

## Problem

There is no integration scenario that drives the action with `INPUT_DRY_RUN=true` against a real FTP server. The smoke tests cover the dry-run path (smoke.sh:447-460 Test 24, plus the resolution of `--dry-run` into MIRROR_COMMAND at smoke.sh:494-501 Test 26), but the smoke harness cannot verify:

1. That lftp 4.9.3's `--dry-run` actually computes the mirror plan without transferring files (smoke only checks the resolved command line).
2. That `--dry-run` + `--delete` reports the deletion list without executing (smoke cannot exercise delete against a real server).
3. That the success banner switches to `FTP DRY RUN COMPLETED` instead of `FTP UPLOADED FINISHED!` (the smoke harness never reaches print_success_banner because lftp always fails on port 1).

## Coverage today

| Path | Smoke (port 1, lftp fails) | Integration |
|---|---|---|
| `INPUT_DRY_RUN=false` (real upload) | partial (asserts MIRROR_COMMAND) | scenario 08, 09, 10, 11, 12 |
| `INPUT_DRY_RUN=true` + dry-run plan | MISSING | MISSING |

The closest existing scenario is scenario 11 (INPUT_EXCLUDE_DELETE end-to-end) which uses `INPUT_DRY_RUN=false`. None of the 11 scenarios pass `INPUT_DRY_RUN=true`.

## Proposed fix

Add a new scenario (e.g. `13-dry-run-with-delete.sh` or `13-dry-run-plan.sh`) that:

1. Boots vsftpd (plain FTP, no SSL) — variant B or variant C is fine.
2. Pre-seeds the FTP user home with one or two files that are NOT in the local fixture (the "would-be-deleted" probe).
3. Runs the action with `INPUT_DELETE=true` + `INPUT_DRY_RUN=true`.
4. Asserts:
- Action exits 0 (dry-run success).
- Captured log contains `FTP DRY RUN COMPLETED` and NOT `FTP UPLOADED FINISHED!`.
- The pre-seeded files are STILL on the server (delete was not executed — only reported).
- The three fixture entries are reported as `would upload` (or similar lftp dry-run output).

Mirrors the smoke test's MIRROR_COMMAND assertion with the lftp side-effect check that only an integration test can provide.

## Severity

LOW — documentation says dry-run is supported; smoke tests the resolved command; missing only the end-to-end "did lftp actually not transfer" assertion. No user data hazard if dry-run silently uploaded.

## Related

- #231 (open): smoke tests do not cover FTPS — same family of integration-coverage gaps
- #230 (open): bats coverage gaps for compute_backoff_seconds and validate_path edge cases
- #167 (open): scenarios 03/04 INPUT_MAX_RETRIES=3 masks TLS flake (orthogonal — but the 03/04 scenario shape is the model to follow for a new scenario)

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.