You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by "tisonkun (via GitHub)" <gi...@apache.org> on 2023/10/04 10:51:46 UTC
[PR] CURATOR-692. Get rid of mockito [curator]
tisonkun opened a new pull request, #485:
URL: https://github.com/apache/curator/pull/485
Ref - CURATOR-692
--
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: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CURATOR-692. Get rid of mockito [curator]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun merged PR #485:
URL: https://github.com/apache/curator/pull/485
--
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: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CURATOR-692. Get rid of mockito [curator]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #485:
URL: https://github.com/apache/curator/pull/485#discussion_r1345599813
##########
curator-framework/src/test/java/org/apache/curator/framework/imps/TestFramework.java:
##########
@@ -755,19 +754,6 @@ public void testNullNamespace() {
CloseableUtils.closeQuietly(client);
}
- @Test
- public void testNoPartialConstruction() {
Review Comment:
The original behavior test is covered by testNullNamespace - https://github.com/apache/curator/pull/475
We don't need this detailed 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: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CURATOR-692. Get rid of mockito [curator]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #485:
URL: https://github.com/apache/curator/pull/485#discussion_r1345601549
##########
curator-client/src/test/java/org/apache/curator/TestEnsurePath.java:
##########
Review Comment:
1. `EnsurePath` is well-implemented, battle-tested, and deprecated, which means it's stable and doesn't not subject to change.
2. This test checks how many `exist` calls happen - why? Although we can take some effort to port it, I don't think it is worthy.
--
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: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CURATOR-692. Get rid of mockito [curator]
Posted by "kezhuw (via GitHub)" <gi...@apache.org>.
kezhuw commented on code in PR #485:
URL: https://github.com/apache/curator/pull/485#discussion_r1354605771
##########
curator-client/pom.xml:
##########
@@ -62,12 +62,6 @@
<artifactId>slf4j-api</artifactId>
</dependency>
- <dependency>
- <groupId>org.mockito</groupId>
- <artifactId>mockito-core</artifactId>
- <scope>test</scope>
- </dependency>
Review Comment:
I am positive to this. I prefer real classes than mocks in most cases if not all. I do believe mock could provide "clean" tests, but the "clean" part could lost effect in refactor comparing to stupid/verbose/explicit hooks. Also, I am fearing about tests full of mocks. Removing this will force us to think about testability. That is good for me.
--
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: commits-unsubscribe@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org