You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "rajucomp (via GitHub)" <gi...@apache.org> on 2024/12/18 13:18:20 UTC
[PR] [Work In Progress] [POOL-238] Improve test generics [commons-pool]
rajucomp opened a new pull request, #374:
URL: https://github.com/apache/commons-pool/pull/374
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
Thanks for your contribution to [Apache Commons](https://commons.apache.org/)! Your help is appreciated!
Before you push a pull request, review this list:
- [ ] Read the [contribution guidelines](CONTRIBUTING.md) for this project.
- [ ] Run a successful build using the default [Maven](https://maven.apache.org/) goal with `mvn`; that's `mvn` on the command line by itself.
- [ ] Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible but is a best-practice.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Each commit in the pull request should have a meaningful subject line and body. Note that commits might be squashed by a maintainer on merge.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Work In Progress] [POOL-238] Improve test generics [commons-pool]
Posted by "rajucomp (via GitHub)" <gi...@apache.org>.
rajucomp commented on code in PR #374:
URL: https://github.com/apache/commons-pool/pull/374#discussion_r1890318467
##########
src/test/java/org/apache/commons/pool3/impl/TestSoftReferenceObjectPool.java:
##########
@@ -26,23 +26,23 @@
*/
public class TestSoftReferenceObjectPool extends TestBaseObjectPool {
- private static final class SimpleFactory extends BasePooledObjectFactory<String, RuntimeException> {
+ private static final class SimpleFactory <T, E extends Exception> extends BasePooledObjectFactory<T, E> {
int counter;
@Override
- public String create() {
- return String.valueOf(counter++);
+ public T create() {
+ return (T)String.valueOf(counter++);
Review Comment:
Yeah. That will be cleaned up.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Work In Progress] [POOL-238] Improve test generics [commons-pool]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #374:
URL: https://github.com/apache/commons-pool/pull/374#issuecomment-2551569463
@rajucomp
I'm not going to spend time reviewing changes to tests that are not required or needed for a bug fix.
If you want to help this component, look at bug fixes.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Work In Progress] [POOL-238] Improve test generics [commons-pool]
Posted by "rajucomp (via GitHub)" <gi...@apache.org>.
rajucomp closed pull request #374: [Work In Progress] [POOL-238] Improve test generics
URL: https://github.com/apache/commons-pool/pull/374
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Work In Progress] [POOL-238] Improve test generics [commons-pool]
Posted by "rajucomp (via GitHub)" <gi...@apache.org>.
rajucomp commented on PR #374:
URL: https://github.com/apache/commons-pool/pull/374#issuecomment-2551308750
@garydgregory I have created a draft pull request for you to review at [https://github.com/apache/commons-pool/pull/374](https://github.com/apache/commons-pool/pull/374.) . There is some finishing touches still required but I want to know if this is something you would consider worth reviewing, Let me know your thoughts on this.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Work In Progress] [POOL-238] Improve test generics [commons-pool]
Posted by "rajucomp (via GitHub)" <gi...@apache.org>.
rajucomp commented on PR #374:
URL: https://github.com/apache/commons-pool/pull/374#issuecomment-2551454214
> Hello @rajucomp I am sorry but this PR is too way big and messy to review. If this is about generics, then ONLY make it about generics please. The PR adds Mockito which is not needed to test generics IMO. There are also cosmetic changes it seems. A PR should be focused. I'd rather see effort in fixing bugs than picking up an 11 year old Jira ticket.
The way the tests are currently being tested is not efficient. We use a class called `MethodCall` to verify function calls which does not seem the right way. Changing the test to use generics will require changes in the `MethodCall` class as well. I could try to shorten the PR to include only minimal generic changes.
Later, I can raise a PR that includes Mockito changes. Does that make sense ?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Work In Progress] [POOL-238] Improve test generics [commons-pool]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #374:
URL: https://github.com/apache/commons-pool/pull/374#discussion_r1890280578
##########
src/test/java/org/apache/commons/pool3/impl/TestSoftReferenceObjectPool.java:
##########
@@ -26,23 +26,23 @@
*/
public class TestSoftReferenceObjectPool extends TestBaseObjectPool {
- private static final class SimpleFactory extends BasePooledObjectFactory<String, RuntimeException> {
+ private static final class SimpleFactory <T, E extends Exception> extends BasePooledObjectFactory<T, E> {
int counter;
@Override
- public String create() {
- return String.valueOf(counter++);
+ public T create() {
+ return (T)String.valueOf(counter++);
Review Comment:
This does not make sense to me. What am I missing? If I declare a SimpleFactory<Integer, RuntimeException>, what happens?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org