slachiewicz opened a new pull request, #904:
URL: https://github.com/apache/maven-wagon/pull/904
The embedded SSH tests have not run in over a decade. This gets the embedded
half of them green and runs it in CI.
### Why the suite could not start
`PlexusTestCase` uses `plexus-container-default`, which reads
`META-INF/plexus/components.xml`. `plexus-interactivity-api` stopped shipping
that descriptor in 1.3 and now ships only a sisu index — I checked the jars:
1.1 has the plexus descriptor and no sisu index, 1.5.1 the reverse. Maven
itself runs on sisu, so nothing is wrong in production; only `PlexusTestCase`
is affected.
The `ComponentLookupException` on the `scp` role hint hid five levels of
cause. The bottom is a missing `Prompter`, which `ConsoleInteractiveUserInfo`
and `PrompterUIKeyboardInteractive` both require and `AbstractJschWagon` needs
both of. All 94 errors had that same bottom cause. It arrived with the 1.1 →
1.3 bump in `42ee3769`.
Writing a descriptor for the real `DefaultPrompter` does not work: it takes
its collaborators through a constructor into final fields, and the container's
builder only does no-arg construction plus field injection. So the tests get
their own `Prompter`, in test scope, which throws rather than blocking on stdin.
### It was three faults, not one
The Prompter alone took 94 errors to 87.
- `ShellCommand` flushed the output stream *after* `ExitCallback.onExit()`
had closed the channel. The server logged `SshChannelClosedException`, the
session went down, and the next command in it died. Removing that one stray
`flush()` took the embedded tests from 19 errors to 6.
- `getExpectedLastModifiedOnGet` compared at millisecond precision while the
scp `T` header carries whole seconds, so the mock transfer event never matched
and the progress assertion saw 0 bytes. Truncated to seconds.
- The embedded server registered no SFTP subsystem, and OpenSSH 9+ drives
`scp` over SFTP, so every shelled-out `scp` exited 255.
### One production change, called out for review
`ScpWagon.fillInputData` mapped only scp ack code 1 to
`ResourceDoesNotExistException`; MINA sshd reports a missing file with code 2.
Without this, four tests stay red.
While there: `line.indexOf("no such file or directory") != 1` is a typo for
`!= -1`, which makes *every* code-1 error a `ResourceDoesNotExistException`. I
deliberately preserved that behaviour rather than quietly tightening it, so
only code 2 is genuinely new. Worth fixing separately, with its own thought
about what it changes.
### Counts
| | before | after |
|---|---|---|
| `wagon-ssh`, `-Dssh-tests -Dssh-embedded=true` | 42 tests, 24 errors |
**42 tests, 0 failures** |
| `wagon-ssh`, `-Dssh-tests` alone | 112 tests, 94 errors | 112 tests, 8
errors + 2 failures |
| full reactor with both switches | — | BUILD SUCCESS |
| affected modules, no switches | — | BUILD SUCCESS |
No test was disabled or deleted.
### Why bare `-Dssh-tests` still cannot be green
By design of those tests, not by breakage. `ScpWagonTest`,
`SshCommandExecutorTest` and `KnownHostsProviderTest` expect a real sshd on
localhost:22 and the developer's own account. `SftpWagonTest` and
`ScpWagonWithSshPrivateKeySearchTest` connect to `scp://localhost:0/`, broken
since `35ff4024` removed `getTestRepositoryPort()`. Only `Embedded*Test` uses
the embedded server, which is exactly what the existing `ssh-embedded` profile
selects — hence the two-property combination in CI.
### CI
```yaml
maven-args: '-D"invoker.streamLogsOnFailures" -Dssh-tests
-Dssh-embedded=true'
```
Two pom changes make that safe, both checked empirically rather than
assumed. Surefire `<excludes>` from two active profiles **override rather than
merge**, last declared winning — verified by running `-Dssh-embedded=true`
without `-Dssh-tests` and watching the Embedded tests run anyway. On Windows
that would have let `ssh-embedded` beat `windauze` and run the
`/bin/sh`-dependent tests, so `ssh-embedded` now also requires a non-Windows
OS. And `wagon-ssh-external`'s own `ssh-embedded` profile would have run a test
that is not green, so it is excluded there with a comment; that module runs no
ssh tests today, so nothing is lost.
If you would rather not carry a switch at all, dropping `**/Embedded*Test.*`
from `no-ssh-tests` gives identical coverage with no workflow change.
### Caveats
Only exercised on JDK 21; the CI matrix includes JDK 8, and these tests are
new to CI on every version. `wagon-ssh-external`'s
`EmbeddedScpExternalWagonWithKeyTest` still fails on path quoting —
`ScpExternalWagon` backslash-escapes spaces for the legacy remote-shell
protocol, which SFTP-mode scp passes through verbatim. That needs a decision
about `-O` versus changed quoting, and the answer depends on the host's OpenSSH
version, so it is left alone here.
### Why this matters beyond itself
This is the only harness that runs the SSH provider against a real server.
It is the precursor to #902 and #903: with it in place, the JSch swap those PRs
make is verified rather than asserted — and it immediately earned that,
catching a `NoClassDefFoundError` that would have stopped `wagon-ssh` loading
at all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
