Testlib fails to test maps with nullable keys and nonnull values

[GoogleCodeExporter]

ALLOW_NULL_KEYS feature implies ALLOW_NULL_QUERIES, which is applied to 
keySet() as well as values(), map.containsValue() and entrySet().

There should be more features:
ALLOW_NULL_KEYS, ALLOW_NULL_KEY_QUERIES, ALLOW_NULL_VALUES, 
ALLOW_NULL_VALUE_QUERIES, ALLOW_NULL_ENTRY_QUERIES

Original issue reported on code.google.com by leventov...@gmail.com on 30 Jan 2014 at 1:01

[GoogleCodeExporter]

Out of curiosity, are there any JDK or Guava collections like this? I agree 
that we're conflating multiple things here. It's just that there are a lot of 
possible tester features, and others have higher priority (like your issue 
1650).

Original comment by cpov...@google.com on 30 Jan 2014 at 2:43

• Added labels: Type-Addition
• Removed labels: Type-Defect

[GoogleCodeExporter]

No, there aren't. But the testlib is not for testing only Guava collections, 
otherwise it won't be distributed as separate jar, isn't it?

It seems to be quite simple to implement, I could even take up this task, if 
you are ready to accept the patch later.

Original comment by leventov...@gmail.com on 30 Jan 2014 at 3:47

[GoogleCodeExporter]

> No, there aren't. But the testlib is not for testing only Guava collections,
> otherwise it won't be distributed as separate jar, isn't it?

Quite so. It's just a question of priorities.

> It seems to be quite simple to implement, I could even take up this task, if
> you are ready to accept the patch later.

...and if you're willing to make it a priority, then it's a lot easier to sell 
:)

Two things to note:

1. You'll need to review the Contributor License Agreement and make sure that 
you can sign it: https://developers.google.com/open-source/cla/individual

2. There's still some chance that the patch would be turned down if the 
complexity is too high. But (without having given it much thought) I wouldn't 
expect much trouble.

Original comment by cpov...@google.com on 30 Jan 2014 at 4:52

[GoogleCodeExporter]

Whether this addition will be introduced in Guava 17?

Original comment by leventov...@gmail.com on 5 Feb 2014 at 8:49

[GoogleCodeExporter]

Do you have unpublished comments and changes in the code review? I haven't seen 
a response there.

https://codereview.appspot.com/58760044/

(Sorry, I should have said something when you asked last week, but somehow I 
thought that someone *else* was asking *you* whether this would be added. My 
mistake.)

Original comment by cpov...@google.com on 13 Feb 2014 at 2:17

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

No, I don't. I had to respond? I thought everything is OK, except backward 
compatibility for guava-testlib users, because this is a pretty serious change.

Original comment by leventov...@gmail.com on 13 Feb 2014 at 11:43

[GoogleCodeExporter]

I had a few requests there for you to take care of. After that, I can pull your 
CL in, make sure it works with our internal users, and get it into Guava proper.

As to backward compatibility, don't worry about it. We consider all of testlib 
to be @Beta.

Original comment by cpov...@google.com on 14 Feb 2014 at 12:00

[GoogleCodeExporter]

Thanks for the patch!

http://code.google.com/p/guava-libraries/source/detail?r=ca1bb340d73748ddd91038b
258a16cf361eb8262

Original comment by cpov...@google.com on 3 Mar 2014 at 3:08

• Changed state: Fixed
• Added labels: Milestone-Release17

[GoogleCodeExporter]

Small fix:

diff --git 
a/guava-testlib/src/com/google/common/collect/testing/testers/MapGetTester.java 
b/guava-testlib/src/com/google/common/collect/testing/testers/MapGetTester.java
index 4670ea4..f416b1f 100644
--- 
a/guava-testlib/src/com/google/common/collect/testing/testers/MapGetTester.java
+++ 
b/guava-testlib/src/com/google/common/collect/testing/testers/MapGetTester.java
@@ -16,9 +16,9 @@

 package com.google.common.collect.testing.testers;

-import static 
com.google.common.collect.testing.features.CollectionFeature.ALLOWS_NULL_QUERIES
;
 import static com.google.common.collect.testing.features.CollectionSize.ZERO;
 import static com.google.common.collect.testing.features.MapFeature.ALLOWS_NULL_KEYS;
+import static 
com.google.common.collect.testing.features.MapFeature.ALLOWS_NULL_KEY_QUERIES;

 import com.google.common.annotations.GwtCompatible;
 import com.google.common.collect.testing.AbstractMapTester;
@@ -47,12 +47,12 @@ public class MapGetTester<K, V> extends 
AbstractMapTester<K, V> {
     assertNull("get(notPresent) should return null", get(samples.e3.getKey()));
   }

-  @CollectionFeature.Require(ALLOWS_NULL_QUERIES)
+  @MapFeature.Require(ALLOWS_NULL_KEY_QUERIES)
   public void testGet_nullNotContainedButAllowed() {
     assertNull("get(null) should return null", get(null));
   }

-  @CollectionFeature.Require(absent = ALLOWS_NULL_QUERIES)
+  @MapFeature.Require(absent = ALLOWS_NULL_KEY_QUERIES)
   public void testGet_nullNotContainedAndUnsupported() {
     try {
       assertNull("get(null) should return null or throw", get(null));

Don't send the patch to codereview app because upload.py don't support python 
3, and I don't want to install python 2.

Original comment by leventov...@gmail.com on 12 Jul 2014 at 10:00

[GoogleCodeExporter]

I'll apply the patch. Thanks again.

Original comment by cpov...@google.com on 21 Jul 2014 at 6:53

[GoogleCodeExporter]

This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<issue id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:10

• Added labels: MigratedToGitHub

[GoogleCodeExporter]

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:07

• Added labels: Restrict-AddIssueComment-Commit