mdedetrich commented on PR #205:
URL:
https://github.com/apache/incubator-pekko-connectors/pull/205#issuecomment-1693385667
@pjfanning @TheDeadOne So I have just committed the changes I was talking
about previously although there are some notable changes I had to make.
One of them is that rather than using a real
`X509ExtendedKeyManager`/`X509ExtendedTrustManager` I instead ended up mocking
it with mockito. Although it was originally working (both on @TheDeadOne 's
version and when I did the initial work), at some point I was getting the
following error
```
[info]
org.apache.pekko.stream.connectors.ftp.FtpsWithTrustAndKeyManagersStageSpec ***
ABORTED *** (1 millisecond)
[info] org.mockito.exceptions.base.MockitoException: Cannot mock/spy class
sun.security.ssl.X509TrustManagerImpl
[info] Mockito cannot mock/spy because :
[info] - final class
[info] at
org.apache.pekko.stream.connectors.ftp.FtpsWithTrustAndKeyManagersStageSpec.setupTrustManager
```
I did look at the implementation of `X509ExtendedKeyManager` and I can
confirm that it is final. I am not exactly sure why in some cases Mockito works
fine when using `spy` and in other cases it complains that its `final` so I
just opted to mock the classes along with the method, i.e.
```scala
doNothing().when(trustManager).checkServerTrusted(any(classOf[Array[X509Certificate]]),
anyString,
any(classOf[Socket]))
```
Note that this doesn't make the test any valid, i.e. if you remove the setup
i.e. `.withTrustManager(trustManager)`/`.withKeyManager(keyManager)` then the
test fails (as expected) since its expecting calls to `checkServerTrusted`.
Likewise if you change `verify(trustManager, atLeastOnce()).checkServerTrusted`
to `verify(trustManager, never()).checkServerTrusted` the tests also fail. One
can also make an argument that this test is more "proper", i.e. the intention
of the test is to make sure that setting the `keyManager`/`trustManager` works.
Testing the actual implementation of `trustManager.checkServerTrusted` can be
considered excessive since its an implementation detail.
Aside from that this version of the test is more comprehensive/accurate,
i.e. it inherits `BaseFtpsSpec` and `CommonFtpStageSpec` which means it does
the entire FTPS test suite and the Mockito `verify` is being injected directly
into the pekko `Source`/`Sink` (where as with the original version `.verify`
was being called at the end of the test)
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
