You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "reschke (via GitHub)" <gi...@apache.org> on 2024/05/02 12:02:23 UTC
[PR] COLLECTIONS-842: deprecate Lists incompatible with Java 21, add new variant of AbstractLinkedList [commons-collections]
reschke opened a new pull request, #485:
URL: https://github.com/apache/commons-collections/pull/485
This is a minimal PR. We may need to improve the deprecation notes.
Coverage tests are failing; we'd need to add test coverage for the new variant, which IMHO requires having an implementation using it.
I've verified that this change would work for Jackrabbit, see https://issues.apache.org/jira/browse/JCR-5049 for details.
--
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] COLLECTIONS-842: deprecate Lists incompatible with Java 21, add new variant of AbstractLinkedList [commons-collections]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #485:
URL: https://github.com/apache/commons-collections/pull/485#issuecomment-2090331407
## [Codecov](https://app.codecov.io/gh/apache/commons-collections/pull/485?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `0%` with `346 lines` in your changes are missing coverage. Please review.
> Project coverage is 79.61%. Comparing base [(`229425c`)](https://app.codecov.io/gh/apache/commons-collections/commit/229425c302bae631570b0a6172beef8ec0bfc606?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`f28225e`)](https://app.codecov.io/gh/apache/commons-collections/pull/485?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 85 commits behind head on master.
| [Files](https://app.codecov.io/gh/apache/commons-collections/pull/485?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...collections4/list/AbstractLinkedListForJava21.java](https://app.codecov.io/gh/apache/commons-collections/pull/485?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fcommons%2Fcollections4%2Flist%2FAbstractLinkedListForJava21.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvY29sbGVjdGlvbnM0L2xpc3QvQWJzdHJhY3RMaW5rZWRMaXN0Rm9ySmF2YTIxLmphdmE=) | 0.00% | [346 Missing :warning: ](https://app.codecov.io/gh/apache/commons-collections/pull/485?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #485 +/- ##
============================================
- Coverage 81.60% 79.61% -2.00%
- Complexity 4745 4756 +11
============================================
Files 295 300 +5
Lines 13751 14100 +349
Branches 2022 2071 +49
============================================
+ Hits 11222 11226 +4
- Misses 1929 2277 +348
+ Partials 600 597 -3
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/commons-collections/pull/485?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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: notifications-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] COLLECTIONS-842: deprecate Lists incompatible with Java 21, add new variant of AbstractLinkedList [commons-collections]
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert commented on PR #485:
URL: https://github.com/apache/commons-collections/pull/485#issuecomment-2095927165
Given that a lot of dynamically generated Junit4 tests were lost in the conversion to JUnit5, we may have to write some more tests just to get this into a working build. My initial investigation into fixing the lost tests discovered only that it is a larger task which is out-of-scope here.
Try pasting this into the current test. These are copied from `CursorableLinkedListTest`:
```java
@Test
@SuppressWarnings("unchecked")
public void testSubList() {
List<E> list = makeObject();
list.add((E) "A");
list.add((E) "B");
list.add((E) "C");
list.add((E) "D");
list.add((E) "E");
assertEquals("[A, B, C, D, E]", list.toString());
assertEquals("[A, B, C, D, E]", list.subList(0, 5).toString());
assertEquals("[B, C, D, E]", list.subList(1, 5).toString());
assertEquals("[C, D, E]", list.subList(2, 5).toString());
assertEquals("[D, E]", list.subList(3, 5).toString());
assertEquals("[E]", list.subList(4, 5).toString());
assertEquals("[]", list.subList(5, 5).toString());
}
@Test
@SuppressWarnings("unchecked")
public void testSubListAddBegin() {
List<E> list = makeObject();
list.add((E) "A");
list.add((E) "B");
list.add((E) "C");
list.add((E) "D");
list.add((E) "E");
final List<E> sublist = list.subList(0, 0);
sublist.add((E) "a");
assertEquals("[a, A, B, C, D, E]", list.toString());
assertEquals("[a]", sublist.toString());
sublist.add((E) "b");
assertEquals("[a, b, A, B, C, D, E]", list.toString());
assertEquals("[a, b]", sublist.toString());
}
@Test
@SuppressWarnings("unchecked")
public void testSubListAddEnd() {
List<E> list = makeObject();
list.add((E) "A");
list.add((E) "B");
list.add((E) "C");
list.add((E) "D");
list.add((E) "E");
final List<E> sublist = list.subList(5, 5);
sublist.add((E) "F");
assertEquals("[A, B, C, D, E, F]", list.toString());
assertEquals("[F]", sublist.toString());
sublist.add((E) "G");
assertEquals("[A, B, C, D, E, F, G]", list.toString());
assertEquals("[F, G]", sublist.toString());
}
@Test
@SuppressWarnings("unchecked")
public void testSubListAddMiddle() {
List<E> list = makeObject();
list.add((E) "A");
list.add((E) "B");
list.add((E) "C");
list.add((E) "D");
list.add((E) "E");
final List<E> sublist = list.subList(1, 3);
sublist.add((E) "a");
assertEquals("[A, B, C, a, D, E]", list.toString());
assertEquals("[B, C, a]", sublist.toString());
sublist.add((E) "b");
assertEquals("[A, B, C, a, b, D, E]", list.toString());
assertEquals("[B, C, a, b]", sublist.toString());
}
@Test
@SuppressWarnings("unchecked")
public void testSubListRemove() {
List<E> list = makeObject();
list.add((E) "A");
list.add((E) "B");
list.add((E) "C");
list.add((E) "D");
list.add((E) "E");
final List<E> sublist = list.subList(1, 4);
assertEquals("[B, C, D]", sublist.toString());
assertEquals("[A, B, C, D, E]", list.toString());
sublist.remove("C");
assertEquals("[B, D]", sublist.toString());
assertEquals("[A, B, D, E]", list.toString());
sublist.remove(1);
assertEquals("[B]", sublist.toString());
assertEquals("[A, B, E]", list.toString());
sublist.clear();
assertEquals("[]", sublist.toString());
assertEquals("[A, E]", list.toString());
}
```
When I added this locally the coverage is above 85% which may be enough to get green lights on the build.
--
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] COLLECTIONS-842: deprecate Lists incompatible with Java 21, add new variant of AbstractLinkedList [commons-collections]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on code in PR #485:
URL: https://github.com/apache/commons-collections/pull/485#discussion_r1589087748
##########
src/test/java/org/apache/commons/collections4/list/AbstractLinkedListForJava21Test.java:
##########
@@ -0,0 +1,216 @@
+/*
+ * 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.
+ */
+package org.apache.commons.collections4.list;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.util.Arrays;
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+/**
+ * Test case for {@link AbstractLinkedListForJava21}.
+ */
+public abstract class AbstractLinkedListForJava21Test<E> extends AbstractListTest<E> {
Review Comment:
Should I rename as well?
--
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] COLLECTIONS-842: deprecate Lists incompatible with Java 21, add new variant of AbstractLinkedList [commons-collections]
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert commented on PR #485:
URL: https://github.com/apache/commons-collections/pull/485#issuecomment-2090443105
I duplicated AbstractLinkedListTest with some modifications:
```java
class AbstractLinkedListForJava21Test<E> extends AbstractListTest<E> {
AbstractLinkedListForJava21Test() {
super(AbstractLinkedListForJava21Test.class.getSimpleName());
}
// ...
@Override
public String getCompatibilityVersion() {
return null;
}
@Override
protected boolean skipSerializedCanonicalTests() {
return true;
}
@Override
public AbstractLinkedListForJava21<E> getCollection() {
return (AbstractLinkedListForJava21<E>) super.getCollection();
}
@Override
public List<E> makeObject() {
return new DefaultAbstractLinkedListForJava21<>();
}
private class DefaultAbstractLinkedListForJava21<E> extends AbstractLinkedListForJava21<E> {
DefaultAbstractLinkedListForJava21() {
init();
}
private void readObject(final ObjectInputStream in) throws IOException, ClassNotFoundException {
in.defaultReadObject();
doReadObject(in);
}
private void writeObject(final ObjectOutputStream out) throws IOException {
out.defaultWriteObject();
doWriteObject(out);
}
}
}
```
Coverage of the new linked list class is 94%. There is no coverage of sublist iterators or creating from a Collection. These may have to be added from other tests of the old AbstractLinkedList, or elsewhere.
I have skipped the canonical serialization test. For this release there is no backward compatibility to maintain.
It is a start towards hitting the coverage thresholds.
--
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] COLLECTIONS-842: deprecate Lists incompatible with Java 21, add new variant of AbstractLinkedList [commons-collections]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on code in PR #485:
URL: https://github.com/apache/commons-collections/pull/485#discussion_r1589087748
##########
src/test/java/org/apache/commons/collections4/list/AbstractLinkedListForJava21Test.java:
##########
@@ -0,0 +1,216 @@
+/*
+ * 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.
+ */
+package org.apache.commons.collections4.list;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.util.Arrays;
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+/**
+ * Test case for {@link AbstractLinkedListForJava21}.
+ */
+public abstract class AbstractLinkedListForJava21Test<E> extends AbstractListTest<E> {
Review Comment:
Should I rename as well?
--
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] COLLECTIONS-842: deprecate Lists incompatible with Java 21, add new variant of AbstractLinkedList [commons-collections]
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert commented on code in PR #485:
URL: https://github.com/apache/commons-collections/pull/485#discussion_r1587705290
##########
src/test/java/org/apache/commons/collections4/list/AbstractLinkedListForJava21Test.java:
##########
@@ -0,0 +1,216 @@
+/*
+ * 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.
+ */
+package org.apache.commons.collections4.list;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.util.Arrays;
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+/**
+ * Test case for {@link AbstractLinkedListForJava21}.
+ */
+public abstract class AbstractLinkedListForJava21Test<E> extends AbstractListTest<E> {
+
+ public AbstractLinkedListForJava21Test() {
+ super(AbstractLinkedListForJava21Test.class.getSimpleName());
+ }
+
+ protected void checkNodes() {
+ final AbstractLinkedListForJava21<E> list = getCollection();
+ for (int i = 0; i < list.size; i++) {
+ assertEquals(list.getNode(i, false).next, list.getNode(i + 1, true));
+ if (i < list.size - 1) {
+ assertEquals(list.getNode(i + 1, false).previous,
+ list.getNode(i, false));
+ }
+ }
+ }
+
+ @Test
+ @SuppressWarnings("unchecked")
+ public void testAddNodeAfter() {
+ resetEmpty();
+ final AbstractLinkedListForJava21<E> list = getCollection();
+ if (!isAddSupported()) {
+ try {
+ list.addFirst(null);
+ } catch (final UnsupportedOperationException ex) {}
+ }
+
+ list.addFirst((E) "value1");
+ list.addNodeAfter(list.getNode(0, false), (E) "value2");
+ assertEquals("value1", list.getFirst());
+ assertEquals("value2", list.getLast());
+ list.removeFirst();
+ checkNodes();
+ list.addNodeAfter(list.getNode(0, false), (E) "value3");
+ checkNodes();
+ assertEquals("value2", list.getFirst());
+ assertEquals("value3", list.getLast());
+ list.addNodeAfter(list.getNode(0, false), (E) "value4");
+ checkNodes();
+ assertEquals("value2", list.getFirst());
+ assertEquals("value3", list.getLast());
+ assertEquals("value4", list.get(1));
+ list.addNodeAfter(list.getNode(2, false), (E) "value5");
+ checkNodes();
+ assertEquals("value2", list.getFirst());
+ assertEquals("value4", list.get(1));
+ assertEquals("value3", list.get(2));
+ assertEquals("value5", list.getLast());
+ }
+
+ @Test
+ @SuppressWarnings("unchecked")
+ public void testGetNode() {
+ resetEmpty();
+ final AbstractLinkedListForJava21<E> list = getCollection();
+ // get marker
+ assertEquals(list.getNode(0, true).previous, list.getNode(0, true).next);
+ assertThrows(IndexOutOfBoundsException.class, () -> list.getNode(0, false),
+ "Expecting IndexOutOfBoundsException.");
+ list.addAll( Arrays.asList((E[]) new String[]{"value1", "value2"}));
+ checkNodes();
+ list.addFirst((E) "value0");
+ checkNodes();
+ list.removeNode(list.getNode(1, false));
+ checkNodes();
+ assertThrows(IndexOutOfBoundsException.class, () -> list.getNode(2, false),
+ "Expecting IndexOutOfBoundsException.");
+ assertThrows(IndexOutOfBoundsException.class, () -> list.getNode(-1, false),
+ "Expecting IndexOutOfBoundsException.");
+ assertThrows(IndexOutOfBoundsException.class, () -> list.getNode(3, true),
+ "Expecting IndexOutOfBoundsException.");
+ }
+
+ @Test
+ @SuppressWarnings("unchecked")
+ public void testRemoveFirst() {
+ resetEmpty();
+ final AbstractLinkedListForJava21<E> list = getCollection();
+ if (!isRemoveSupported()) {
+ try {
+ list.removeFirst();
+ } catch (final UnsupportedOperationException ex) {}
+ }
+
+ list.addAll(Arrays.asList((E[]) new String[] { "value1", "value2" }));
+ assertEquals("value1", list.removeFirst());
+ checkNodes();
+ list.addLast((E) "value3");
+ checkNodes();
+ assertEquals("value2", list.removeFirst());
+ assertEquals("value3", list.removeFirst());
+ checkNodes();
+ list.addLast((E) "value4");
+ checkNodes();
+ assertEquals("value4", list.removeFirst());
+ checkNodes();
+ }
+
+ @Test
+ @SuppressWarnings("unchecked")
+ public void testRemoveLast() {
+ resetEmpty();
+ final AbstractLinkedListForJava21<E> list = getCollection();
+ if (!isRemoveSupported()) {
+ try {
+ list.removeLast();
+ } catch (final UnsupportedOperationException ex) {}
+ }
+
+ list.addAll(Arrays.asList((E[]) new String[] { "value1", "value2" }));
+ assertEquals("value2", list.removeLast());
+ list.addFirst((E) "value3");
+ checkNodes();
+ assertEquals("value1", list.removeLast());
+ assertEquals("value3", list.removeLast());
+ list.addFirst((E) "value4");
+ checkNodes();
+ assertEquals("value4", list.removeFirst());
+ }
+
+ @Test
+ @SuppressWarnings("unchecked")
+ public void testRemoveNode() {
+ resetEmpty();
+ if (!isAddSupported() || !isRemoveSupported()) {
+ return;
+ }
+ final AbstractLinkedListForJava21<E> list = getCollection();
+
+ list.addAll(Arrays.asList((E[]) new String[] { "value1", "value2" }));
+ list.removeNode(list.getNode(0, false));
+ checkNodes();
+ assertEquals("value2", list.getFirst());
+ assertEquals("value2", list.getLast());
+ list.addFirst((E) "value1");
+ list.addFirst((E) "value0");
+ checkNodes();
+ list.removeNode(list.getNode(1, false));
+ assertEquals("value0", list.getFirst());
+ assertEquals("value2", list.getLast());
+ checkNodes();
+ list.removeNode(list.getNode(1, false));
+ assertEquals("value0", list.getFirst());
+ assertEquals("value0", list.getLast());
+ checkNodes();
+ }
+
+ @Override
+ public String getCompatibilityVersion() {
+ return null;
+ }
+
+ @Override
+ protected boolean skipSerializedCanonicalTests() {
+ return true;
+ }
+
+ @Override
+ public AbstractLinkedListForJava21<E> getCollection() {
+ return (AbstractLinkedListForJava21<E>) super.getCollection();
+ }
+
+ @Override
+ public List<E> makeObject() {
+ return new DefaultAbstractLinkedListForJava21<>();
+ }
+
+ private class DefaultAbstractLinkedListForJava21<E> extends AbstractLinkedListForJava21<E> {
Review Comment:
This class should be static. Sorry for my cut and paste error. Otherwise the serialization test triggers serialization of the enclosing class AbstractLinkedListForJava21Test (which is not serializable).
--
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] COLLECTIONS-842: deprecate Lists incompatible with Java 21, add new variant of AbstractLinkedList [commons-collections]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on PR #485:
URL: https://github.com/apache/commons-collections/pull/485#issuecomment-2096510411
@aherbert - thanks for helping out!
--
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] COLLECTIONS-842: deprecate Lists incompatible with Java 21, add new variant of AbstractLinkedList [commons-collections]
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert commented on code in PR #485:
URL: https://github.com/apache/commons-collections/pull/485#discussion_r1589033479
##########
src/test/java/org/apache/commons/collections4/list/AbstractLinkedListForJava21Test.java:
##########
@@ -0,0 +1,216 @@
+/*
+ * 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.
+ */
+package org.apache.commons.collections4.list;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.util.Arrays;
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+/**
+ * Test case for {@link AbstractLinkedListForJava21}.
+ */
+public abstract class AbstractLinkedListForJava21Test<E> extends AbstractListTest<E> {
Review Comment:
This test class should not be abstract. JUnit will not run any tests.
--
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] COLLECTIONS-842: deprecate Lists incompatible with Java 21, add new variant of AbstractLinkedList [commons-collections]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on PR #485:
URL: https://github.com/apache/commons-collections/pull/485#issuecomment-2090447676
@aherbert - excellent; do you want to push to my repo, or should we move over to the commons-collections git repo?
--
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] COLLECTIONS-842: deprecate Lists incompatible with Java 21, add new variant of AbstractLinkedList [commons-collections]
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert commented on PR #485:
URL: https://github.com/apache/commons-collections/pull/485#issuecomment-2093082324
I only tested this with:
```
mvn test -Dtest=AbstractLinkedListForJava21Test jacoco:report
```
It seems that if you just run the default `test` phase then the class is not run (I presume this is due to the name Abstract as a prefix). I renamed to `DefaultAbstractLinkedListForJava21Test` and then it does run.
We do not hit usage of the sub-list with these tests.
It seems that this functionality was removed in COLLECTIONS-806 when converting from JUnit4 to JUnit5. Previously `o.a.c.collections4.BulkTest` (the base for all tests) was used to recursively generate test fixtures via the custom `makeSuite` method which was called from within a method used by JUnit4 e.g.:
```java
// in CursorableLinkedListTest
public static junit.framework.Test suite() {
return BulkTest.makeSuite(CursorableLinkedListTest.class);
}
```
The javadoc of `BulkTest` refers to this method but commit b860d0c removed this functionality.
IIUC that commit has removed a large amount of test functionality by dropping the dynamic generation of tests for sub-collections.
We cannot fix this now. It should be discussed elsewhere (e.g. on the mailing list). So let's just get this test running and hopefully enough coverage to pass the thresholds.
--
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] COLLECTIONS-842: deprecate Lists incompatible with Java 21, add new variant of AbstractLinkedList [commons-collections]
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert commented on PR #485:
URL: https://github.com/apache/commons-collections/pull/485#issuecomment-2090453664
Just add this mod to your repo and it will be collected here for the PR. Another maintainer may wish to pass an eye over it.
--
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] COLLECTIONS-842: deprecate Lists incompatible with Java 21, add new variant of AbstractLinkedList [commons-collections]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on PR #485:
URL: https://github.com/apache/commons-collections/pull/485#issuecomment-2095601759
Ths helped, but still fails with:
~~~
[WARNING] Rule violated for bundle commons-collections4: instructions covered ratio is 0.84, but expected minimum is 0.85
~~~
--
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] COLLECTIONS-842: deprecate Lists incompatible with Java 21, add new variant of AbstractLinkedList [commons-collections]
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert commented on code in PR #485:
URL: https://github.com/apache/commons-collections/pull/485#discussion_r1592405933
##########
src/main/java/org/apache/commons/collections4/list/CursorableLinkedList.java:
##########
@@ -57,7 +57,9 @@
*
* @see java.util.LinkedList
* @since 1.0
+ * @deprecated
Review Comment:
All the other deprecated tags in the library have a comment on what to use instead. We currently do not have an alternative but we should add something useful here to note why it is deprecated:
```java
@deprecated parent {@link AbstractLinkedList} is source incompatible with List methods added in Java 21
```
If a request comes to add a replacement then it can be done under a new enhancement.
##########
src/main/java/org/apache/commons/collections4/list/NodeCachingLinkedList.java:
##########
@@ -40,7 +40,9 @@
* </p>
*
* @since 3.0
+ * @deprecated
Review Comment:
Same comment here
--
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] COLLECTIONS-842: deprecate Lists incompatible with Java 21, add new variant of AbstractLinkedList [commons-collections]
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert merged PR #485:
URL: https://github.com/apache/commons-collections/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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org