Cleanup null validation when configuring image #768
Conversation
Also cleans up some tests and moves them around
|
|
||
| @Test | ||
| @SuppressWarnings("JdkObsolete") | ||
| public void testBuilder_nullValues() { |
There was a problem hiding this comment.
These tests were moved to ContainerConfigurationTest
| ContainerConfiguration.builder().setEnvironment(ImmutableMap.of()).build()); | ||
|
|
||
| // Environment map can accept TreeMap and Hashtable. | ||
| BuildConfiguration.builder(Mockito.mock(JibLogger.class)) |
There was a problem hiding this comment.
These were removed, doesn't make sense to ensure TreeMap/HashMap are valid Map types.
There was a problem hiding this comment.
Now I remember: #632 (comment)
The point of the test was to ensure that we don't call things like containsKey(null), because while ImmutableMap is OK with that, some Map implementations like TreeMap throws an exception right away. Maybe adding the issue link as a comment could make this clearer.
There was a problem hiding this comment.
that's a good point, I'll put them back in.
| // Exposed ports element should not be null. | ||
| try { | ||
| ContainerConfiguration.builder() | ||
| .setExposedPorts(Arrays.asList(new Port(1000, Protocol.TCP), null)); |
There was a problem hiding this comment.
This is still not narrow enough. For example, suppose Port was modified to throw IllegalArgumentException when given 1000. The test will still pass if we introduce a bug into .setExposedPorts() that won't throw the exception.
There was a problem hiding this comment.
Ah right. Will update
| @@ -0,0 +1,76 @@ | |||
| package com.google.cloud.tools.jib.configuration; | |||
|
|
|||
| import static org.junit.Assert.*; | |||
There was a problem hiding this comment.
looks like there's a static wildcard import here.
| @@ -0,0 +1,76 @@ | |||
| package com.google.cloud.tools.jib.configuration; | |||
| import org.junit.Assert; | ||
| import org.junit.Test; | ||
|
|
||
| public class ContainerConfigurationTest { |
loosebazooka
changed the title
|
I pushed the "Delete Branch" button. You can always restore it later. |
* Cleanup null validation when configuration image
This is just a followup to #750 to cleanup and organize some code