nickwallen commented on a change in pull request #1458: METRON-2177 Upgrade
Profiler for HBase 2.0.2
URL: https://github.com/apache/metron/pull/1458#discussion_r311176523
##########
File path:
metron-analytics/metron-profiler-client/src/test/java/org/apache/metron/profiler/client/stellar/GetProfileTest.java
##########
@@ -20,437 +20,182 @@
package org.apache.metron.profiler.client.stellar;
-import org.apache.hadoop.hbase.client.HTableInterface;
-import org.apache.metron.hbase.mock.MockHBaseTableProvider;
import org.apache.metron.profiler.ProfileMeasurement;
-import org.apache.metron.profiler.client.ProfileWriter;
-import org.apache.metron.profiler.hbase.ColumnBuilder;
-import org.apache.metron.profiler.hbase.RowKeyBuilder;
-import org.apache.metron.profiler.hbase.SaltyRowKeyBuilder;
-import org.apache.metron.profiler.hbase.ValueOnlyColumnBuilder;
+import org.apache.metron.profiler.client.ProfilerClient;
import org.apache.metron.stellar.common.DefaultStellarStatefulExecutor;
import org.apache.metron.stellar.common.StellarStatefulExecutor;
import org.apache.metron.stellar.dsl.Context;
-import org.apache.metron.stellar.dsl.ParseException;
+import org.apache.metron.stellar.dsl.functions.resolver.FunctionResolver;
import org.apache.metron.stellar.dsl.functions.resolver.SimpleFunctionResolver;
-import
org.apache.metron.stellar.dsl.functions.resolver.SingletonFunctionResolver;
-import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
-import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.concurrent.TimeUnit;
-import static
org.apache.metron.profiler.client.stellar.ProfilerClientConfig.PROFILER_COLUMN_FAMILY;
-import static
org.apache.metron.profiler.client.stellar.ProfilerClientConfig.PROFILER_HBASE_TABLE;
-import static
org.apache.metron.profiler.client.stellar.ProfilerClientConfig.PROFILER_HBASE_TABLE_PROVIDER;
-import static
org.apache.metron.profiler.client.stellar.ProfilerClientConfig.PROFILER_PERIOD;
-import static
org.apache.metron.profiler.client.stellar.ProfilerClientConfig.PROFILER_PERIOD_UNITS;
-import static
org.apache.metron.profiler.client.stellar.ProfilerClientConfig.PROFILER_SALT_DIVISOR;
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
/**
- * Tests the GetProfile class.
+ * Tests the 'PROFILE_GET' function in the {@link GetProfile} class.
*/
public class GetProfileTest {
- private static final long periodDuration = 15;
- private static final TimeUnit periodUnits = TimeUnit.MINUTES;
- private static final int saltDivisor = 1000;
- private static final String tableName = "profiler";
- private static final String columnFamily = "P";
private StellarStatefulExecutor executor;
- private Map<String, Object> state;
- private ProfileWriter profileWriter;
- // different values of period and salt divisor, used to test
config_overrides feature
- private static final long periodDuration2 = 1;
- private static final TimeUnit periodUnits2 = TimeUnit.HOURS;
- private static final int saltDivisor2 = 2050;
-
- private <T> T run(String expression, Class<T> clazz) {
- return executor.execute(expression, state, clazz);
+ private FunctionResolver functionResolver;
+ private Map<String, Object> globals;
+ private GetProfile function;
+ private ProfilerClient profilerClient;
+ private List<Integer> results;
+ private ProfileMeasurement expected;
+
+ private List run(String expression) {
+ return executor.execute(expression, new HashMap<>(), List.class);
}
- /**
- * This method sets up the configuration context for both writing profile
data
- * (using profileWriter to mock the complex process of what the Profiler
topology
- * actually does), and then reading that profile data (thereby testing the
PROFILE_GET
- * Stellar client implemented in GetProfile).
- *
- * It runs at @Before time, and sets testclass global variables used by the
writers and readers.
- * The various writers and readers are in each test case, not here.
- *
- * @return void
- */
@Before
public void setup() {
- state = new HashMap<>();
- final HTableInterface table = MockHBaseTableProvider.addToCache(tableName,
columnFamily);
-
- // used to write values to be read during testing
- long periodDurationMillis = TimeUnit.MINUTES.toMillis(15);
- RowKeyBuilder rowKeyBuilder = new SaltyRowKeyBuilder();
- ColumnBuilder columnBuilder = new ValueOnlyColumnBuilder(columnFamily);
- profileWriter = new ProfileWriter(rowKeyBuilder, columnBuilder, table,
periodDurationMillis);
-
- // global properties
- Map<String, Object> global = new HashMap<String, Object>() {{
- put(PROFILER_HBASE_TABLE.getKey(), tableName);
- put(PROFILER_COLUMN_FAMILY.getKey(), columnFamily);
- put(PROFILER_HBASE_TABLE_PROVIDER.getKey(),
MockHBaseTableProvider.class.getName());
- put(PROFILER_PERIOD.getKey(), Long.toString(periodDuration));
- put(PROFILER_PERIOD_UNITS.getKey(), periodUnits.toString());
- put(PROFILER_SALT_DIVISOR.getKey(), Integer.toString(saltDivisor));
- }};
+ // the mock profiler client used to feed profile measurement values to the
function
+ profilerClient = mock(ProfilerClient.class);
- // create the stellar execution environment
- executor = new DefaultStellarStatefulExecutor(
- new SimpleFunctionResolver()
- .withClass(GetProfile.class)
- .withClass(FixedLookback.class),
- new Context.Builder()
- .with(Context.Capabilities.GLOBAL_CONFIG, () -> global)
- .build());
- }
-
- /**
- * This method is similar to setup(), in that it sets up profiler
configuration context,
- * but only for the client. Additionally, it uses periodDuration2,
periodUnits2
- * and saltDivisor2, instead of periodDuration, periodUnits and saltDivisor
respectively.
- *
- * This is used in the unit tests that test the config_overrides feature of
PROFILE_GET.
- * In these tests, the context from @Before setup() is used to write the
data, then the global
- * context is changed to context2 (from this method). Each test validates
that a default read
- * using global context2 then gets no valid results (as expected), and that
a read using
- * original context values in the PROFILE_GET config_overrides argument gets
all expected results.
- *
- * @return context2 - The profiler client configuration context created by
this method.
- * The context2 values are also set in the configuration of the
StellarStatefulExecutor
- * stored in the global variable 'executor'. However, there is no API
for querying the
- * context values from a StellarStatefulExecutor, so we output the
context2 Context object itself,
- * for validation purposes (so that its values can be validated as being
significantly
- * different from the setup() settings).
- */
- private Context setup2() {
- state = new HashMap<>();
+ // the VERBOSE_PROFILE function that will be tested
+ function = new GetProfile();
Review comment:
I just find this approach more clear without having to write a separate
Builder class that would require more code.
Just to highlight what I mean by "more clear." For example, I think this
code in the tests is more clear...
```
function = new GetProfile();
function.withProfilerClientFactory(globals -> profilerClient);
```
Than this...
```
function = new GetProfile(globals -> profilerClient);
```
When I look at the use of a lambda as a constructor argument, I am left
asking myself, "what the heck is a `globals -> profilerClient`"? When you see
the first code example, you immediately know that the lambda is a
`ProfilerClientFactory`.
Now I could get the same clarity by creating a separate Builder class as you
mentioned. But I have to write more code to do a Builder; maybe just tens of
lines, but it is more code. What are the benefits of using a Builder here?
I'll do it if the benefit is clear. I can see the benefits of a Builder when
there are lots of things that need plugged in, but here it is just once; a
`ProfilerClientFactory`.
----------------------------------------------------------------
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
