mmiklavc commented on a change in pull request #1399: METRON-2073: Create
in-memory use case for enrichment with map type and flatfile summarizer
URL: https://github.com/apache/metron/pull/1399#discussion_r286600711
##########
File path:
metron-platform/metron-enrichment/metron-enrichment-common/src/test/java/org/apache/metron/enrichment/stellar/ObjectGetTest.java
##########
@@ -18,73 +18,88 @@
package org.apache.metron.enrichment.stellar;
-import com.google.common.collect.ImmutableMap;
-import org.apache.commons.io.IOUtils;
-import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.fs.FileSystem;
-import org.apache.hadoop.fs.Path;
-import org.apache.metron.common.utils.SerDeUtils;
-import org.apache.metron.stellar.common.utils.StellarProcessorUtils;
-import org.junit.Assert;
+import org.apache.metron.enrichment.cache.ObjectCache;
+import org.apache.metron.enrichment.cache.ObjectCacheConfig;
+import org.apache.metron.stellar.dsl.Context;
import org.junit.Before;
+import org.junit.Rule;
import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.runner.RunWith;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
-import java.io.BufferedOutputStream;
-import java.io.IOException;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
-import java.util.List;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+import static org.powermock.api.mockito.PowerMockito.whenNew;
+
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({ObjectGet.class, ObjectCache.class})
public class ObjectGetTest {
- FileSystem fs;
- List<String> data;
+ @Rule
+ public ExpectedException thrown = ExpectedException.none();
+
+ private ObjectGet objectGet;
+ private ObjectCache objectCache;
+ private Context context;
@Before
- public void setup() throws IOException {
- fs = FileSystem.get(new Configuration());
- data = new ArrayList<>();
- {
- data.add("apache");
- data.add("metron");
- data.add("is");
- data.add("great");
- }
+ public void setup() throws Exception {
+ objectGet = new ObjectGet();
+ objectCache = mock(ObjectCache.class);
Review comment:
Any reason not to just use the real ObjectCache here? I get that it might
arguably be an integration test at that point, but it's also a pretty trivial
dependency. I've been thinking about this a while now, and I think we have
quite a few instances where we're doing dependency injection and mocking where
just using the real objects would probably be 1) simpler 2) clearer and 3) a
more accurate, less error-prone test. This test is simple enough, though there
are many other where the test setup for the mocks is anything but obvious. For
example, this is from a recent PR of mine -
https://github.com/apache/metron/pull/1409/files#diff-ab0ebf385d42a0fe97232a3e1131936bR338.
More often than not, I find that spies (parserRunner ->
https://github.com/apache/metron/pull/1409/files#diff-ab0ebf385d42a0fe97232a3e1131936bR222)
are a symptom/code smell that warrants some further thought on what a class is
doing and whether we should be pushing some of its responsibility to other
collaborating classes.
See some expanded thoughts on mocking from Uncle Bob Martin's here -
https://blog.cleancoder.com/uncle-bob/2014/05/10/WhenToMock.html
Per another of my comments, it looks like the main reason to mock here is
the underlying Loader. I'd consider injecting a simple faux `Loader` if you
don't want to add that complexity. I think there's value in testing the true
integration between the Stellar function `ENRICHMENT_OBJECT_GET` wrapper and
the underlying `ObjectCache` implementation versus just mocking the
functionality of `ObjectCache`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
