feat: add implementation for Version Protocol Service Stub #1760
Conversation
… into feat/implement_mock_version_service
|
Hi @bmg13, unfortunately, I had to break your nicely updated DEPENDENCIES file 😞 |
… into feat/implement_mock_version_service
… into feat/implement_mock_version_service
| assertThat(value.contains("version=2024/1, path=/2024/1")); | ||
| assertThat(value.contains("version=v0.8, path=/")); |
There was a problem hiding this comment.
these aren't asserting anything, they should be written as:
assertThat(value).contains("version=2024/1, path=/2024/1");
assertThat(value).contains("version=v0.8, path=/");however, this is not correct anyway, because the result should be a json (ref.), the assertions could also be made with restassured directly (example)
There was a problem hiding this comment.
you're right, thank you! Kept the assertions separated, but now as the result as json, as expected. Changed in here.
|
|
||
| import java.util.concurrent.CompletableFuture; | ||
|
|
||
| public class VersionServiceStub extends AbstractServiceStub implements VersionService { |
There was a problem hiding this comment.
this is an api exposed on the protocol, it does not need to be part of this mock connector (only management)
There was a problem hiding this comment.
I understand, however the VersionService is the one we wan to mock in ProtocolVersionApiV4AlphaController. Which means, the ProtcolApiVersionExtension needs this mock to not have an wiring issue.
What I believe can (and should) be changed is removing the VersionProtocolServiceStub since is not needed because is never invoked, i.e., the controller for VersionProtocol does not need the mock of VersionProtocol.
Tested locally without the VersionProtocolServiceStub just to confirm.
There was a problem hiding this comment.
yes, sure, it was an oversight on my side. VersionProtocolService does not need to be mocked
| @@ -38,7 +38,6 @@ dependencies { | |||
| runtimeOnly(libs.edc.boot) | |||
| runtimeOnly(libs.edc.api.management) { | |||
| exclude("org.eclipse.edc", "edr-cache-api") | |||
There was a problem hiding this comment.
this makes me think that we should add also the edr-cache-api endpoint, could you open an issue about?
There was a problem hiding this comment.
good point, created in here.
… into feat/implement_mock_version_service
|
WHAT
Adds implementation for a stub Version Protocol Service, similar to existent ones.
WHY
To allow mocking the protocol api.
FURTHER NOTES
Helpful context on this approach: #1264
Closes ##1733