Hello core-libs. I was wondering if somebody could please review the
following updated webrev for this bug. There was an e-mail thread
regarding the first webrev [1] that prompted the changes in this webrev.
Webrev: http://cr.openjdk.java.net/~mduigou/4802647/1/webrev/
Bug URL: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4802647
Here is a summary of the changes:
1. I followed the suggestion to look into Collection/MOAT.java and
decided that updating this test was better than using the new test
program I had written in the previous webrev named
RemoveAllRetainAllNPE.java
2. I noticed that MOAT.java didn't directly test AbstractCollection.java
and AbstractSet.java. So, I added the testing of these classes to
MOAT.java on lines 62 and 63 of the updated webrev.
3. While MOAT.java does test removeAll(null) and retainAll(null), it doesn't
test the scenario where the collection is empty when calling c.removeAll(null)
and c.retainAll(null). The current testing in MOAT.java uses a collection with
one element when testing removeAll(null) and retainAll(null). So, I added code
for the testing of removeAll(null) and retainAll(null) with an empty
collection, which is the scenario for the behavior in the bug report. These
are the changes on lines 759 - 766, 775 - 782 of MOAT.java.
4. The added classes NewAbstractColection and NewAbstractSet were intentionally
written to be very basic and were inspired from the source in the bug report.
Is there a better way to write these classes or are they ok as-is?
5. The changes to MOAT.java in this updated webrev result in more classes that
exhibit the same bug behavior described in this bug report for
AbstractCollection (no NPE thrown). These classes are: java.util.ArrayList and
java.util.concurrent.CopyOnWriteArrayList. Should a new bug be filed for these
cases?
Thanks.
[1]:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2011-November/008283.html
On 12/2/2011 2:34 AM, Alan Bateman wrote:
On 01/12/2011 22:42, Brandon Passanisi wrote:
Hi Jason. Thanks for your response. I was thinking about how I can
improve the test using your suggestion. I could possibly do the
following:
1. Find all of the subclasses of AbstractCollection which override
removeAll(Collection<?>) and which also contain the spec language
which specifies that NullPointerException is thrown if the specified
collection is null.
2. Find all of the subclasses of AbstractCollection which override
retainAll(Collection<?>) and which also contain the spec language
which specifies that NullPointerException is thrown if the specified
collection is null.
3. Add the classes found in #1 and #2 to the test.
4. If any of the new classes added because of #3 result in a test
failure, it might be a good idea to file a new bug as Bug #4802647
specifically mentions subclasses of AbstractCollection which do not
override remainAll, retainAll.
5. The public subclasses of AbstractCollection which do not override
removeAll, retainAll (probably) shouldn't be included in the test as
the currently existing NewAbstractCollection represents this
scenario.
What do you think?
Brandon - it's probably worth getting familiar with the existing
tests, in in particular Martin's "Mother Of All Test"
(Collection/MOAT.java).
-Alan.
--
Oracle <http://www.oracle.com>
Brandon Passanisi | Principle Member of Technical Staff
Green Oracle <http://www.oracle.com/commitment> Oracle is committed to
developing practices and products that help protect the environment
