Details

    • Type: New Feature
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.8.0
    • Component/s: test
    • Labels:
      None

      Description

      Users often write iterators without fully understanding its limits and lifetime. Accumulo should have an iterator fuzz-tester which will take user data and run the iterator under extreme conditions. For example, it should re-create and re-seek the iterator with every key returned. It could automatically compare results of such a run with the naive run, which seeks to the beginning and scans all the data.

        Issue Links

          Activity

          Hide
          elserj Josh Elser added a comment -

          It would be cool to use the MiniAccumuloCluster to implement this to let people include a "fuzzer" into their own projects and test their iterators.

          Show
          elserj Josh Elser added a comment - It would be cool to use the MiniAccumuloCluster to implement this to let people include a "fuzzer" into their own projects and test their iterators.
          Hide
          elserj Josh Elser added a comment -

          Stealin' from you, sir John Vines

          Show
          elserj Josh Elser added a comment - Stealin' from you, sir John Vines
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user joshelser opened a pull request:

          https://github.com/apache/accumulo/pull/50

          ACCUMULO-626 Iterator fuzzer/tester

          Some early code for automatically testing iterators.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/joshelser/accumulo ACCUMULO-626-fuzzer

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/accumulo/pull/50.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #50


          commit 768a118c492229463df5c8c361c33559d92b7508
          Author: Josh Elser <elserj@apache.org>
          Date: 2015-11-07T19:17:17Z

          ACCUMULO-626 Initial outline of the test harness.

          commit fecf28ae30e845457e62a13efca5be83b4fc04ef
          Author: Josh Elser <elserj@apache.org>
          Date: 2015-11-07T21:09:17Z

          Create a basic JUnit4 test case that wraps IteratorTestRunner.

          commit 21b3ce14dc4199a3b0974d33ac04b0767543554d
          Author: Josh Elser <elserj@apache.org>
          Date: 2015-11-07T22:22:13Z

          Some initial tests against WholeRowIterator

          commit 11bf87f8b13a45f82f04a5f6617e55fc0af4dea5
          Author: Josh Elser <elserj@apache.org>
          Date: 2015-11-07T22:31:58Z

          Add a base TestCase that performs verification on the TestOutput.

          commit 2f71336d683c94106997a2e2ed2c2471c8c6b707
          Author: Josh Elser <elserj@apache.org>
          Date: 2015-11-07T22:52:30Z

          Add a utility to dynamically find all provided IteratorTestCases.

          commit b00a1b794b1d0787cdb43185b917b5e7b7b5cff0
          Author: Josh Elser <elserj@apache.org>
          Date: 2015-11-08T08:03:44Z

          DeepCopy and ReSeek test cases.

          commit 273f1f5c6e026a2bc0810e9f5bf22a7c98bf9e13
          Author: Josh Elser <elserj@apache.org>
          Date: 2015-11-08T08:05:57Z

          More docs.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user joshelser opened a pull request: https://github.com/apache/accumulo/pull/50 ACCUMULO-626 Iterator fuzzer/tester Some early code for automatically testing iterators. You can merge this pull request into a Git repository by running: $ git pull https://github.com/joshelser/accumulo ACCUMULO-626 -fuzzer Alternatively you can review and apply these changes as the patch at: https://github.com/apache/accumulo/pull/50.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #50 commit 768a118c492229463df5c8c361c33559d92b7508 Author: Josh Elser <elserj@apache.org> Date: 2015-11-07T19:17:17Z ACCUMULO-626 Initial outline of the test harness. commit fecf28ae30e845457e62a13efca5be83b4fc04ef Author: Josh Elser <elserj@apache.org> Date: 2015-11-07T21:09:17Z Create a basic JUnit4 test case that wraps IteratorTestRunner. commit 21b3ce14dc4199a3b0974d33ac04b0767543554d Author: Josh Elser <elserj@apache.org> Date: 2015-11-07T22:22:13Z Some initial tests against WholeRowIterator commit 11bf87f8b13a45f82f04a5f6617e55fc0af4dea5 Author: Josh Elser <elserj@apache.org> Date: 2015-11-07T22:31:58Z Add a base TestCase that performs verification on the TestOutput. commit 2f71336d683c94106997a2e2ed2c2471c8c6b707 Author: Josh Elser <elserj@apache.org> Date: 2015-11-07T22:52:30Z Add a utility to dynamically find all provided IteratorTestCases. commit b00a1b794b1d0787cdb43185b917b5e7b7b5cff0 Author: Josh Elser <elserj@apache.org> Date: 2015-11-08T08:03:44Z DeepCopy and ReSeek test cases. commit 273f1f5c6e026a2bc0810e9f5bf22a7c98bf9e13 Author: Josh Elser <elserj@apache.org> Date: 2015-11-08T08:05:57Z More docs.
          Hide
          elserj Josh Elser added a comment -
          Show
          elserj Josh Elser added a comment - Here's an early pull request with an example of some tests over WholeRowIterator. JUnit case for WRI: https://github.com/joshelser/accumulo/blob/ACCUMULO-626-fuzzer/iterator-test-harness/src/test/java/org/apache/accumulo/iteratortest/WholeRowIteratorTest.java Generic "test" for verifying that re-seeking works in an iterator: https://github.com/joshelser/accumulo/blob/ACCUMULO-626-fuzzer/iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/testcases/ReSeekTestCase.java Needs polishing, but feedback welcome.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user madrob commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r44291578

          — Diff: iterator-test-harness/pom.xml —
          @@ -0,0 +1,56 @@
          +<?xml version="1.0" encoding="UTF-8"?>
          +<!--
          + Licensed to the Apache Software Foundation (ASF) under one or more
          + contributor license agreements. See the NOTICE file distributed with
          + this work for additional information regarding copyright ownership.
          + The ASF licenses this file to You under the Apache License, Version 2.0
          + (the "License"); you may not use this file except in compliance with
          + the License. You may obtain a copy of the License at
          +
          + http://www.apache.org/licenses/LICENSE-2.0
          +
          + Unless required by applicable law or agreed to in writing, software
          + distributed under the License is distributed on an "AS IS" BASIS,
          + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + See the License for the specific language governing permissions and
          + limitations under the License.
          +-->
          +<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
          + <modelVersion>4.0.0</modelVersion>
          + <parent>
          + <groupId>org.apache.accumulo</groupId>
          + <artifactId>accumulo-project</artifactId>
          + <version>1.8.0-SNAPSHOT</version>
          + </parent>
          + <artifactId>accumulo-iterator-test-harness</artifactId>
          + <name>Apache Accumulo Iterator Test Harness</name>
          + <description>A library for testing Apache Accumulo Iterators.</description>
          + <dependencies>
          + <!-- TODO Don't force downstream users to have JUnit -->
          + <dependency>
          + <groupId>junit</groupId>
          + <artifactId>junit</artifactId>
          + <!-optional>true</optional->
          + </dependency>
          + <dependency>
          + <groupId>log4j</groupId>
          + <artifactId>log4j</artifactId>
          + </dependency>
          + <dependency>
          + <groupId>org.apache.accumulo</groupId>
          + <artifactId>accumulo-core</artifactId>
          + </dependency>
          + <dependency>
          + <groupId>org.apache.hadoop</groupId>
          + <artifactId>hadoop-client</artifactId>
          + </dependency>
          + <dependency>
          + <groupId>org.reflections</groupId>
          + <artifactId>reflections</artifactId>
          + </dependency>
          — End diff –

          Do we need to add this to the NOTICE?

          Show
          githubbot ASF GitHub Bot added a comment - Github user madrob commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r44291578 — Diff: iterator-test-harness/pom.xml — @@ -0,0 +1,56 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd "> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>org.apache.accumulo</groupId> + <artifactId>accumulo-project</artifactId> + <version>1.8.0-SNAPSHOT</version> + </parent> + <artifactId>accumulo-iterator-test-harness</artifactId> + <name>Apache Accumulo Iterator Test Harness</name> + <description>A library for testing Apache Accumulo Iterators.</description> + <dependencies> + <!-- TODO Don't force downstream users to have JUnit --> + <dependency> + <groupId>junit</groupId> + <artifactId>junit</artifactId> + <!- optional>true</optional -> + </dependency> + <dependency> + <groupId>log4j</groupId> + <artifactId>log4j</artifactId> + </dependency> + <dependency> + <groupId>org.apache.accumulo</groupId> + <artifactId>accumulo-core</artifactId> + </dependency> + <dependency> + <groupId>org.apache.hadoop</groupId> + <artifactId>hadoop-client</artifactId> + </dependency> + <dependency> + <groupId>org.reflections</groupId> + <artifactId>reflections</artifactId> + </dependency> — End diff – Do we need to add this to the NOTICE?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user joshelser commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r44291969

          — Diff: iterator-test-harness/pom.xml —
          @@ -0,0 +1,56 @@
          +<?xml version="1.0" encoding="UTF-8"?>
          +<!--
          + Licensed to the Apache Software Foundation (ASF) under one or more
          + contributor license agreements. See the NOTICE file distributed with
          + this work for additional information regarding copyright ownership.
          + The ASF licenses this file to You under the Apache License, Version 2.0
          + (the "License"); you may not use this file except in compliance with
          + the License. You may obtain a copy of the License at
          +
          + http://www.apache.org/licenses/LICENSE-2.0
          +
          + Unless required by applicable law or agreed to in writing, software
          + distributed under the License is distributed on an "AS IS" BASIS,
          + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + See the License for the specific language governing permissions and
          + limitations under the License.
          +-->
          +<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
          + <modelVersion>4.0.0</modelVersion>
          + <parent>
          + <groupId>org.apache.accumulo</groupId>
          + <artifactId>accumulo-project</artifactId>
          + <version>1.8.0-SNAPSHOT</version>
          + </parent>
          + <artifactId>accumulo-iterator-test-harness</artifactId>
          + <name>Apache Accumulo Iterator Test Harness</name>
          + <description>A library for testing Apache Accumulo Iterators.</description>
          + <dependencies>
          + <!-- TODO Don't force downstream users to have JUnit -->
          + <dependency>
          + <groupId>junit</groupId>
          + <artifactId>junit</artifactId>
          + <!-optional>true</optional->
          + </dependency>
          + <dependency>
          + <groupId>log4j</groupId>
          + <artifactId>log4j</artifactId>
          + </dependency>
          + <dependency>
          + <groupId>org.apache.accumulo</groupId>
          + <artifactId>accumulo-core</artifactId>
          + </dependency>
          + <dependency>
          + <groupId>org.apache.hadoop</groupId>
          + <artifactId>hadoop-client</artifactId>
          + </dependency>
          + <dependency>
          + <groupId>org.reflections</groupId>
          + <artifactId>reflections</artifactId>
          + </dependency>
          — End diff –

          uhhh license is http://www.wtfpl.net/. Ideally, this will be ripped out before it's committed.

          Show
          githubbot ASF GitHub Bot added a comment - Github user joshelser commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r44291969 — Diff: iterator-test-harness/pom.xml — @@ -0,0 +1,56 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd "> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>org.apache.accumulo</groupId> + <artifactId>accumulo-project</artifactId> + <version>1.8.0-SNAPSHOT</version> + </parent> + <artifactId>accumulo-iterator-test-harness</artifactId> + <name>Apache Accumulo Iterator Test Harness</name> + <description>A library for testing Apache Accumulo Iterators.</description> + <dependencies> + <!-- TODO Don't force downstream users to have JUnit --> + <dependency> + <groupId>junit</groupId> + <artifactId>junit</artifactId> + <!- optional>true</optional -> + </dependency> + <dependency> + <groupId>log4j</groupId> + <artifactId>log4j</artifactId> + </dependency> + <dependency> + <groupId>org.apache.accumulo</groupId> + <artifactId>accumulo-core</artifactId> + </dependency> + <dependency> + <groupId>org.apache.hadoop</groupId> + <artifactId>hadoop-client</artifactId> + </dependency> + <dependency> + <groupId>org.reflections</groupId> + <artifactId>reflections</artifactId> + </dependency> — End diff – uhhh license is http://www.wtfpl.net/ . Ideally, this will be ripped out before it's committed.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user keith-turner commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r44310769

          — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/IteratorTestCaseFinder.java —
          @@ -0,0 +1,61 @@
          +/*
          + * Licensed to the Apache Software Foundation (ASF) under one or more
          + * contributor license agreements. See the NOTICE file distributed with
          + * this work for additional information regarding copyright ownership.
          + * The ASF licenses this file to you under the Apache License, Version 2.0
          + * (the "License"); you may not use this file except in compliance with
          + * the License. You may obtain a copy of the License at
          + *
          + * http://www.apache.org/licenses/LICENSE-2.0
          + *
          + * Unless required by applicable law or agreed to in writing, software
          + * distributed under the License is distributed on an "AS IS" BASIS,
          + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + * See the License for the specific language governing permissions and
          + * limitations under the License.
          + */
          +package org.apache.accumulo.iteratortest;
          +
          +import java.lang.reflect.Modifier;
          +import java.util.ArrayList;
          +import java.util.List;
          +import java.util.Set;
          +
          +import org.apache.accumulo.iteratortest.testcases.IteratorTestCase;
          +import org.reflections.Reflections;
          +import org.slf4j.Logger;
          +import org.slf4j.LoggerFactory;
          +
          +/**
          + * A class to ease finding published test cases.
          + */
          +public class IteratorTestCaseFinder {
          + private static final Logger log = LoggerFactory.getLogger(IteratorTestCaseFinder.class);
          +
          + /**
          + * Instantiates all test cases provided.
          + *
          + * @return A list of

          {@link IteratorTestCase}

          s.
          + */
          + public static List<IteratorTestCase> findAllTestCases() {
          + log.info("Searching {}", IteratorTestCase.class.getPackage().getName());
          + Reflections reflections = new Reflections(IteratorTestCase.class.getPackage().getName());
          — End diff –

          If this is a new dependency, maybe you could use Guava's [ClassPath.getAllClasses()][1]. Could iterator an filter based on type.

          [1]: http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/reflect/ClassPath.html#getAllClasses%28%29

          Show
          githubbot ASF GitHub Bot added a comment - Github user keith-turner commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r44310769 — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/IteratorTestCaseFinder.java — @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.iteratortest; + +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; + +import org.apache.accumulo.iteratortest.testcases.IteratorTestCase; +import org.reflections.Reflections; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * A class to ease finding published test cases. + */ +public class IteratorTestCaseFinder { + private static final Logger log = LoggerFactory.getLogger(IteratorTestCaseFinder.class); + + /** + * Instantiates all test cases provided. + * + * @return A list of {@link IteratorTestCase} s. + */ + public static List<IteratorTestCase> findAllTestCases() { + log.info("Searching {}", IteratorTestCase.class.getPackage().getName()); + Reflections reflections = new Reflections(IteratorTestCase.class.getPackage().getName()); — End diff – If this is a new dependency, maybe you could use Guava's [ClassPath.getAllClasses()] [1] . Could iterator an filter based on type. [1] : http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/reflect/ClassPath.html#getAllClasses%28%29
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user keith-turner commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r44311672

          — Diff: iterator-test-harness/src/test/java/org/apache/accumulo/iteratortest/WholeRowIteratorTest.java —
          @@ -0,0 +1,147 @@
          +/*
          + * Licensed to the Apache Software Foundation (ASF) under one or more
          + * contributor license agreements. See the NOTICE file distributed with
          + * this work for additional information regarding copyright ownership.
          + * The ASF licenses this file to you under the Apache License, Version 2.0
          + * (the "License"); you may not use this file except in compliance with
          + * the License. You may obtain a copy of the License at
          + *
          + * http://www.apache.org/licenses/LICENSE-2.0
          + *
          + * Unless required by applicable law or agreed to in writing, software
          + * distributed under the License is distributed on an "AS IS" BASIS,
          + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + * See the License for the specific language governing permissions and
          + * limitations under the License.
          + */
          +package org.apache.accumulo.iteratortest;
          +
          +import java.io.IOException;
          +import java.util.ArrayList;
          +import java.util.Collections;
          +import java.util.List;
          +import java.util.Map.Entry;
          +import java.util.TreeMap;
          +
          +import org.apache.accumulo.core.data.Key;
          +import org.apache.accumulo.core.data.Range;
          +import org.apache.accumulo.core.data.Value;
          +import org.apache.accumulo.core.iterators.user.WholeRowIterator;
          +import org.apache.accumulo.iteratortest.junit4.BaseJUnit4IteratorTest;
          +import org.apache.accumulo.iteratortest.testcases.IteratorTestCase;
          +import org.apache.hadoop.io.Text;
          +import org.junit.runners.Parameterized.Parameters;
          +
          +/**
          + * Framework tests for

          {@link WholeRowIterator}

          .
          + */
          +public class WholeRowIteratorTest extends BaseJUnit4IteratorTest {
          +
          + @Parameters
          + public static Object[][] parameters()

          { + IteratorTestInput input = getIteratorInput(); + IteratorTestOutput output = getIteratorOutput(); + List<IteratorTestCase> tests = IteratorTestCaseFinder.findAllTestCases(); + return BaseJUnit4IteratorTest.createParameters(input, output, tests); + }

          +
          + private static final TreeMap<Key,Value> INPUT_DATA = createInputData();
          + private static final TreeMap<Key,Value> OUTPUT_DATA = createOutputData();
          +
          + private static TreeMap<Key,Value> createInputData()

          { + TreeMap<Key,Value> data = new TreeMap<>(); + + data.put(new Key("1", "", "a"), new Value("1a".getBytes())); + data.put(new Key("1", "", "b"), new Value("1b".getBytes())); + data.put(new Key("1", "a", "a"), new Value("1aa".getBytes())); + data.put(new Key("1", "a", "b"), new Value("1ab".getBytes())); + data.put(new Key("1", "b", "a"), new Value("1ba".getBytes())); + + data.put(new Key("2", "a", "a"), new Value("2aa".getBytes())); + data.put(new Key("2", "a", "b"), new Value("2ab".getBytes())); + data.put(new Key("2", "a", "c"), new Value("2ac".getBytes())); + data.put(new Key("2", "c", "c"), new Value("2cc".getBytes())); + + data.put(new Key("3", "a", ""), new Value("3a".getBytes())); + + data.put(new Key("4", "a", "b"), new Value("4ab".getBytes())); + + data.put(new Key("5", "a", "a"), new Value("5aa".getBytes())); + data.put(new Key("5", "a", "b"), new Value("5ab".getBytes())); + data.put(new Key("5", "a", "c"), new Value("5ac".getBytes())); + data.put(new Key("5", "a", "d"), new Value("5ad".getBytes())); + + data.put(new Key("6", "", "a"), new Value("6a".getBytes())); + data.put(new Key("6", "", "b"), new Value("6b".getBytes())); + data.put(new Key("6", "", "c"), new Value("6c".getBytes())); + data.put(new Key("6", "", "d"), new Value("6d".getBytes())); + data.put(new Key("6", "", "e"), new Value("6e".getBytes())); + data.put(new Key("6", "1", "a"), new Value("61a".getBytes())); + data.put(new Key("6", "1", "b"), new Value("61b".getBytes())); + data.put(new Key("6", "1", "c"), new Value("61c".getBytes())); + data.put(new Key("6", "1", "d"), new Value("61d".getBytes())); + data.put(new Key("6", "1", "e"), new Value("61e".getBytes())); + + return data; + }

          +
          + private static TreeMap<Key,Value> createOutputData() {
          + TreeMap<Key,Value> data = new TreeMap<>();
          +
          + Text row = null;
          + List<Key> keys = new ArrayList<>();
          + List<Value> values = new ArrayList<>();
          +
          + // Generate the output data from the input data
          + for (Entry<Key,Value> entry : INPUT_DATA.entrySet()) {
          — End diff –

          Using [RowIterator](https://accumulo.apache.org/1.6/apidocs/org/apache/accumulo/core/client/RowIterator.html) may simplify this code a bit.

          Show
          githubbot ASF GitHub Bot added a comment - Github user keith-turner commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r44311672 — Diff: iterator-test-harness/src/test/java/org/apache/accumulo/iteratortest/WholeRowIteratorTest.java — @@ -0,0 +1,147 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.iteratortest; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map.Entry; +import java.util.TreeMap; + +import org.apache.accumulo.core.data.Key; +import org.apache.accumulo.core.data.Range; +import org.apache.accumulo.core.data.Value; +import org.apache.accumulo.core.iterators.user.WholeRowIterator; +import org.apache.accumulo.iteratortest.junit4.BaseJUnit4IteratorTest; +import org.apache.accumulo.iteratortest.testcases.IteratorTestCase; +import org.apache.hadoop.io.Text; +import org.junit.runners.Parameterized.Parameters; + +/** + * Framework tests for {@link WholeRowIterator} . + */ +public class WholeRowIteratorTest extends BaseJUnit4IteratorTest { + + @Parameters + public static Object[][] parameters() { + IteratorTestInput input = getIteratorInput(); + IteratorTestOutput output = getIteratorOutput(); + List<IteratorTestCase> tests = IteratorTestCaseFinder.findAllTestCases(); + return BaseJUnit4IteratorTest.createParameters(input, output, tests); + } + + private static final TreeMap<Key,Value> INPUT_DATA = createInputData(); + private static final TreeMap<Key,Value> OUTPUT_DATA = createOutputData(); + + private static TreeMap<Key,Value> createInputData() { + TreeMap<Key,Value> data = new TreeMap<>(); + + data.put(new Key("1", "", "a"), new Value("1a".getBytes())); + data.put(new Key("1", "", "b"), new Value("1b".getBytes())); + data.put(new Key("1", "a", "a"), new Value("1aa".getBytes())); + data.put(new Key("1", "a", "b"), new Value("1ab".getBytes())); + data.put(new Key("1", "b", "a"), new Value("1ba".getBytes())); + + data.put(new Key("2", "a", "a"), new Value("2aa".getBytes())); + data.put(new Key("2", "a", "b"), new Value("2ab".getBytes())); + data.put(new Key("2", "a", "c"), new Value("2ac".getBytes())); + data.put(new Key("2", "c", "c"), new Value("2cc".getBytes())); + + data.put(new Key("3", "a", ""), new Value("3a".getBytes())); + + data.put(new Key("4", "a", "b"), new Value("4ab".getBytes())); + + data.put(new Key("5", "a", "a"), new Value("5aa".getBytes())); + data.put(new Key("5", "a", "b"), new Value("5ab".getBytes())); + data.put(new Key("5", "a", "c"), new Value("5ac".getBytes())); + data.put(new Key("5", "a", "d"), new Value("5ad".getBytes())); + + data.put(new Key("6", "", "a"), new Value("6a".getBytes())); + data.put(new Key("6", "", "b"), new Value("6b".getBytes())); + data.put(new Key("6", "", "c"), new Value("6c".getBytes())); + data.put(new Key("6", "", "d"), new Value("6d".getBytes())); + data.put(new Key("6", "", "e"), new Value("6e".getBytes())); + data.put(new Key("6", "1", "a"), new Value("61a".getBytes())); + data.put(new Key("6", "1", "b"), new Value("61b".getBytes())); + data.put(new Key("6", "1", "c"), new Value("61c".getBytes())); + data.put(new Key("6", "1", "d"), new Value("61d".getBytes())); + data.put(new Key("6", "1", "e"), new Value("61e".getBytes())); + + return data; + } + + private static TreeMap<Key,Value> createOutputData() { + TreeMap<Key,Value> data = new TreeMap<>(); + + Text row = null; + List<Key> keys = new ArrayList<>(); + List<Value> values = new ArrayList<>(); + + // Generate the output data from the input data + for (Entry<Key,Value> entry : INPUT_DATA.entrySet()) { — End diff – Using [RowIterator] ( https://accumulo.apache.org/1.6/apidocs/org/apache/accumulo/core/client/RowIterator.html ) may simplify this code a bit.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user joshelser commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r44311676

          — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/IteratorTestCaseFinder.java —
          @@ -0,0 +1,61 @@
          +/*
          + * Licensed to the Apache Software Foundation (ASF) under one or more
          + * contributor license agreements. See the NOTICE file distributed with
          + * this work for additional information regarding copyright ownership.
          + * The ASF licenses this file to you under the Apache License, Version 2.0
          + * (the "License"); you may not use this file except in compliance with
          + * the License. You may obtain a copy of the License at
          + *
          + * http://www.apache.org/licenses/LICENSE-2.0
          + *
          + * Unless required by applicable law or agreed to in writing, software
          + * distributed under the License is distributed on an "AS IS" BASIS,
          + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + * See the License for the specific language governing permissions and
          + * limitations under the License.
          + */
          +package org.apache.accumulo.iteratortest;
          +
          +import java.lang.reflect.Modifier;
          +import java.util.ArrayList;
          +import java.util.List;
          +import java.util.Set;
          +
          +import org.apache.accumulo.iteratortest.testcases.IteratorTestCase;
          +import org.reflections.Reflections;
          +import org.slf4j.Logger;
          +import org.slf4j.LoggerFactory;
          +
          +/**
          + * A class to ease finding published test cases.
          + */
          +public class IteratorTestCaseFinder {
          + private static final Logger log = LoggerFactory.getLogger(IteratorTestCaseFinder.class);
          +
          + /**
          + * Instantiates all test cases provided.
          + *
          + * @return A list of

          {@link IteratorTestCase}

          s.
          + */
          + public static List<IteratorTestCase> findAllTestCases() {
          + log.info("Searching {}", IteratorTestCase.class.getPackage().getName());
          + Reflections reflections = new Reflections(IteratorTestCase.class.getPackage().getName());
          — End diff –

          Yeah, like I hinted at above, this was a quick hack to get this functionality and I hope to remove it (despite being a very nice library to use). Ideally, I'd not need any extra dependencies to do it (currently don't have any deps outside of accumulo-core, junit and slf4j that I recall), and I'd prefer to leave it that way. If it's hard to do with regular reflection libs, Guava sounds like a nice choice. Thanks for the recommendation.

          Show
          githubbot ASF GitHub Bot added a comment - Github user joshelser commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r44311676 — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/IteratorTestCaseFinder.java — @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.iteratortest; + +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; + +import org.apache.accumulo.iteratortest.testcases.IteratorTestCase; +import org.reflections.Reflections; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * A class to ease finding published test cases. + */ +public class IteratorTestCaseFinder { + private static final Logger log = LoggerFactory.getLogger(IteratorTestCaseFinder.class); + + /** + * Instantiates all test cases provided. + * + * @return A list of {@link IteratorTestCase} s. + */ + public static List<IteratorTestCase> findAllTestCases() { + log.info("Searching {}", IteratorTestCase.class.getPackage().getName()); + Reflections reflections = new Reflections(IteratorTestCase.class.getPackage().getName()); — End diff – Yeah, like I hinted at above, this was a quick hack to get this functionality and I hope to remove it (despite being a very nice library to use). Ideally, I'd not need any extra dependencies to do it (currently don't have any deps outside of accumulo-core, junit and slf4j that I recall), and I'd prefer to leave it that way. If it's hard to do with regular reflection libs, Guava sounds like a nice choice. Thanks for the recommendation.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user madrob commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r44312116

          — Diff: iterator-test-harness/pom.xml —
          @@ -0,0 +1,56 @@
          +<?xml version="1.0" encoding="UTF-8"?>
          +<!--
          + Licensed to the Apache Software Foundation (ASF) under one or more
          + contributor license agreements. See the NOTICE file distributed with
          + this work for additional information regarding copyright ownership.
          + The ASF licenses this file to You under the Apache License, Version 2.0
          + (the "License"); you may not use this file except in compliance with
          + the License. You may obtain a copy of the License at
          +
          + http://www.apache.org/licenses/LICENSE-2.0
          +
          + Unless required by applicable law or agreed to in writing, software
          + distributed under the License is distributed on an "AS IS" BASIS,
          + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + See the License for the specific language governing permissions and
          + limitations under the License.
          +-->
          +<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
          + <modelVersion>4.0.0</modelVersion>
          + <parent>
          + <groupId>org.apache.accumulo</groupId>
          + <artifactId>accumulo-project</artifactId>
          + <version>1.8.0-SNAPSHOT</version>
          + </parent>
          + <artifactId>accumulo-iterator-test-harness</artifactId>
          + <name>Apache Accumulo Iterator Test Harness</name>
          + <description>A library for testing Apache Accumulo Iterators.</description>
          + <dependencies>
          + <!-- TODO Don't force downstream users to have JUnit -->
          + <dependency>
          + <groupId>junit</groupId>
          + <artifactId>junit</artifactId>
          + <!-optional>true</optional->
          + </dependency>
          + <dependency>
          + <groupId>log4j</groupId>
          + <artifactId>log4j</artifactId>
          + </dependency>
          + <dependency>
          + <groupId>org.apache.accumulo</groupId>
          + <artifactId>accumulo-core</artifactId>
          + </dependency>
          + <dependency>
          + <groupId>org.apache.hadoop</groupId>
          + <artifactId>hadoop-client</artifactId>
          + </dependency>
          + <dependency>
          + <groupId>org.reflections</groupId>
          + <artifactId>reflections</artifactId>
          + </dependency>
          — End diff –

          LEGAL-135 and http://www.apache.org/legal/resolved.html#category-a indicate that it's fine. I just wanted to know if we need to document the new dependency somehow.

          Show
          githubbot ASF GitHub Bot added a comment - Github user madrob commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r44312116 — Diff: iterator-test-harness/pom.xml — @@ -0,0 +1,56 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd "> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>org.apache.accumulo</groupId> + <artifactId>accumulo-project</artifactId> + <version>1.8.0-SNAPSHOT</version> + </parent> + <artifactId>accumulo-iterator-test-harness</artifactId> + <name>Apache Accumulo Iterator Test Harness</name> + <description>A library for testing Apache Accumulo Iterators.</description> + <dependencies> + <!-- TODO Don't force downstream users to have JUnit --> + <dependency> + <groupId>junit</groupId> + <artifactId>junit</artifactId> + <!- optional>true</optional -> + </dependency> + <dependency> + <groupId>log4j</groupId> + <artifactId>log4j</artifactId> + </dependency> + <dependency> + <groupId>org.apache.accumulo</groupId> + <artifactId>accumulo-core</artifactId> + </dependency> + <dependency> + <groupId>org.apache.hadoop</groupId> + <artifactId>hadoop-client</artifactId> + </dependency> + <dependency> + <groupId>org.reflections</groupId> + <artifactId>reflections</artifactId> + </dependency> — End diff – LEGAL-135 and http://www.apache.org/legal/resolved.html#category-a indicate that it's fine. I just wanted to know if we need to document the new dependency somehow.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user wjsl commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r44312285

          — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/IteratorTestUtil.java —
          @@ -0,0 +1,44 @@
          +/*
          + * Licensed to the Apache Software Foundation (ASF) under one or more
          + * contributor license agreements. See the NOTICE file distributed with
          + * this work for additional information regarding copyright ownership.
          + * The ASF licenses this file to you under the Apache License, Version 2.0
          + * (the "License"); you may not use this file except in compliance with
          + * the License. You may obtain a copy of the License at
          + *
          + * http://www.apache.org/licenses/LICENSE-2.0
          + *
          + * Unless required by applicable law or agreed to in writing, software
          + * distributed under the License is distributed on an "AS IS" BASIS,
          + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + * See the License for the specific language governing permissions and
          + * limitations under the License.
          + */
          +package org.apache.accumulo.iteratortest;
          +
          +import java.util.Objects;
          +
          +import org.apache.accumulo.core.data.Key;
          +import org.apache.accumulo.core.data.Value;
          +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
          +import org.apache.accumulo.core.iterators.SortedMapIterator;
          +import org.apache.accumulo.core.iterators.system.ColumnFamilySkippingIterator;
          +import org.apache.accumulo.iteratortest.testcases.IteratorTestCase;
          +
          +/**
          + * A collection of methods that are helpful to the development of

          {@link IteratorTestCase}

          s.
          + */
          +public class IteratorTestUtil {
          +
          + public static SortedKeyValueIterator<Key,Value> instantiateIterator(IteratorTestInput input) {
          + try

          { + return Objects.requireNonNull(input.getIteratorClass()).newInstance(); + }

          catch (InstantiationException | IllegalAccessException e)

          { + throw new RuntimeException(e); + }

          + }
          +
          + public static SortedKeyValueIterator<Key,Value> createSource(IteratorTestInput input) {
          + return new ColumnFamilySkippingIterator(new SortedMapIterator(Objects.requireNonNull(input).getInput()));
          — End diff –

          Might be good to make another iterator here (or wrapping the SMI) that re-uses the Key and Value instances returned by the getters. Storing the output of a source in a list or map without making a copy is a common pitfall.

          I had a small iterator benchmark that I modified to printout the identify of each Key and Value, and currently notice the same Value instance has the potential to be re-used. I will try to verify if Keys exhibit the same behavior-- I have heard anecdotes that they do.

          Show
          githubbot ASF GitHub Bot added a comment - Github user wjsl commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r44312285 — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/IteratorTestUtil.java — @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.iteratortest; + +import java.util.Objects; + +import org.apache.accumulo.core.data.Key; +import org.apache.accumulo.core.data.Value; +import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +import org.apache.accumulo.core.iterators.SortedMapIterator; +import org.apache.accumulo.core.iterators.system.ColumnFamilySkippingIterator; +import org.apache.accumulo.iteratortest.testcases.IteratorTestCase; + +/** + * A collection of methods that are helpful to the development of {@link IteratorTestCase} s. + */ +public class IteratorTestUtil { + + public static SortedKeyValueIterator<Key,Value> instantiateIterator(IteratorTestInput input) { + try { + return Objects.requireNonNull(input.getIteratorClass()).newInstance(); + } catch (InstantiationException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } + + public static SortedKeyValueIterator<Key,Value> createSource(IteratorTestInput input) { + return new ColumnFamilySkippingIterator(new SortedMapIterator(Objects.requireNonNull(input).getInput())); — End diff – Might be good to make another iterator here (or wrapping the SMI) that re-uses the Key and Value instances returned by the getters. Storing the output of a source in a list or map without making a copy is a common pitfall. I had a small iterator benchmark that I modified to printout the identify of each Key and Value, and currently notice the same Value instance has the potential to be re-used. I will try to verify if Keys exhibit the same behavior-- I have heard anecdotes that they do.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user joshelser commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r44312491

          — Diff: iterator-test-harness/pom.xml —
          @@ -0,0 +1,56 @@
          +<?xml version="1.0" encoding="UTF-8"?>
          +<!--
          + Licensed to the Apache Software Foundation (ASF) under one or more
          + contributor license agreements. See the NOTICE file distributed with
          + this work for additional information regarding copyright ownership.
          + The ASF licenses this file to You under the Apache License, Version 2.0
          + (the "License"); you may not use this file except in compliance with
          + the License. You may obtain a copy of the License at
          +
          + http://www.apache.org/licenses/LICENSE-2.0
          +
          + Unless required by applicable law or agreed to in writing, software
          + distributed under the License is distributed on an "AS IS" BASIS,
          + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + See the License for the specific language governing permissions and
          + limitations under the License.
          +-->
          +<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
          + <modelVersion>4.0.0</modelVersion>
          + <parent>
          + <groupId>org.apache.accumulo</groupId>
          + <artifactId>accumulo-project</artifactId>
          + <version>1.8.0-SNAPSHOT</version>
          + </parent>
          + <artifactId>accumulo-iterator-test-harness</artifactId>
          + <name>Apache Accumulo Iterator Test Harness</name>
          + <description>A library for testing Apache Accumulo Iterators.</description>
          + <dependencies>
          + <!-- TODO Don't force downstream users to have JUnit -->
          + <dependency>
          + <groupId>junit</groupId>
          + <artifactId>junit</artifactId>
          + <!-optional>true</optional->
          + </dependency>
          + <dependency>
          + <groupId>log4j</groupId>
          + <artifactId>log4j</artifactId>
          + </dependency>
          + <dependency>
          + <groupId>org.apache.accumulo</groupId>
          + <artifactId>accumulo-core</artifactId>
          + </dependency>
          + <dependency>
          + <groupId>org.apache.hadoop</groupId>
          + <artifactId>hadoop-client</artifactId>
          + </dependency>
          + <dependency>
          + <groupId>org.reflections</groupId>
          + <artifactId>reflections</artifactId>
          + </dependency>
          — End diff –

          No worries, your attention to that detail is appreciated. It was really just a "google, copy/paste" job to keep me moving on that I didn't mean to keep long-term.

          Show
          githubbot ASF GitHub Bot added a comment - Github user joshelser commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r44312491 — Diff: iterator-test-harness/pom.xml — @@ -0,0 +1,56 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd "> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>org.apache.accumulo</groupId> + <artifactId>accumulo-project</artifactId> + <version>1.8.0-SNAPSHOT</version> + </parent> + <artifactId>accumulo-iterator-test-harness</artifactId> + <name>Apache Accumulo Iterator Test Harness</name> + <description>A library for testing Apache Accumulo Iterators.</description> + <dependencies> + <!-- TODO Don't force downstream users to have JUnit --> + <dependency> + <groupId>junit</groupId> + <artifactId>junit</artifactId> + <!- optional>true</optional -> + </dependency> + <dependency> + <groupId>log4j</groupId> + <artifactId>log4j</artifactId> + </dependency> + <dependency> + <groupId>org.apache.accumulo</groupId> + <artifactId>accumulo-core</artifactId> + </dependency> + <dependency> + <groupId>org.apache.hadoop</groupId> + <artifactId>hadoop-client</artifactId> + </dependency> + <dependency> + <groupId>org.reflections</groupId> + <artifactId>reflections</artifactId> + </dependency> — End diff – No worries, your attention to that detail is appreciated. It was really just a "google, copy/paste" job to keep me moving on that I didn't mean to keep long-term.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user keith-turner commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r44316155

          — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/IteratorTestUtil.java —
          @@ -0,0 +1,44 @@
          +/*
          + * Licensed to the Apache Software Foundation (ASF) under one or more
          + * contributor license agreements. See the NOTICE file distributed with
          + * this work for additional information regarding copyright ownership.
          + * The ASF licenses this file to you under the Apache License, Version 2.0
          + * (the "License"); you may not use this file except in compliance with
          + * the License. You may obtain a copy of the License at
          + *
          + * http://www.apache.org/licenses/LICENSE-2.0
          + *
          + * Unless required by applicable law or agreed to in writing, software
          + * distributed under the License is distributed on an "AS IS" BASIS,
          + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + * See the License for the specific language governing permissions and
          + * limitations under the License.
          + */
          +package org.apache.accumulo.iteratortest;
          +
          +import java.util.Objects;
          +
          +import org.apache.accumulo.core.data.Key;
          +import org.apache.accumulo.core.data.Value;
          +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
          +import org.apache.accumulo.core.iterators.SortedMapIterator;
          +import org.apache.accumulo.core.iterators.system.ColumnFamilySkippingIterator;
          +import org.apache.accumulo.iteratortest.testcases.IteratorTestCase;
          +
          +/**
          + * A collection of methods that are helpful to the development of

          {@link IteratorTestCase}

          s.
          + */
          +public class IteratorTestUtil {
          +
          + public static SortedKeyValueIterator<Key,Value> instantiateIterator(IteratorTestInput input) {
          + try

          { + return Objects.requireNonNull(input.getIteratorClass()).newInstance(); + }

          catch (InstantiationException | IllegalAccessException e)

          { + throw new RuntimeException(e); + }

          + }
          +
          + public static SortedKeyValueIterator<Key,Value> createSource(IteratorTestInput input) {
          + return new ColumnFamilySkippingIterator(new SortedMapIterator(Objects.requireNonNull(input).getInput()));
          — End diff –

          b. loss did in this in TransformingIteratorTest. See line 662 in 1.6.4(https://github.com/apache/accumulo/blob/1.6.4/core/src/test/java/org/apache/accumulo/core/iterators/user/TransformingIteratorTest.java#L662)

          Show
          githubbot ASF GitHub Bot added a comment - Github user keith-turner commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r44316155 — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/IteratorTestUtil.java — @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.iteratortest; + +import java.util.Objects; + +import org.apache.accumulo.core.data.Key; +import org.apache.accumulo.core.data.Value; +import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +import org.apache.accumulo.core.iterators.SortedMapIterator; +import org.apache.accumulo.core.iterators.system.ColumnFamilySkippingIterator; +import org.apache.accumulo.iteratortest.testcases.IteratorTestCase; + +/** + * A collection of methods that are helpful to the development of {@link IteratorTestCase} s. + */ +public class IteratorTestUtil { + + public static SortedKeyValueIterator<Key,Value> instantiateIterator(IteratorTestInput input) { + try { + return Objects.requireNonNull(input.getIteratorClass()).newInstance(); + } catch (InstantiationException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } + + public static SortedKeyValueIterator<Key,Value> createSource(IteratorTestInput input) { + return new ColumnFamilySkippingIterator(new SortedMapIterator(Objects.requireNonNull(input).getInput())); — End diff – b. loss did in this in TransformingIteratorTest. See line 662 in 1.6.4 ( https://github.com/apache/accumulo/blob/1.6.4/core/src/test/java/org/apache/accumulo/core/iterators/user/TransformingIteratorTest.java#L662 )
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user joshelser commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r44316590

          — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/IteratorTestUtil.java —
          @@ -0,0 +1,44 @@
          +/*
          + * Licensed to the Apache Software Foundation (ASF) under one or more
          + * contributor license agreements. See the NOTICE file distributed with
          + * this work for additional information regarding copyright ownership.
          + * The ASF licenses this file to you under the Apache License, Version 2.0
          + * (the "License"); you may not use this file except in compliance with
          + * the License. You may obtain a copy of the License at
          + *
          + * http://www.apache.org/licenses/LICENSE-2.0
          + *
          + * Unless required by applicable law or agreed to in writing, software
          + * distributed under the License is distributed on an "AS IS" BASIS,
          + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + * See the License for the specific language governing permissions and
          + * limitations under the License.
          + */
          +package org.apache.accumulo.iteratortest;
          +
          +import java.util.Objects;
          +
          +import org.apache.accumulo.core.data.Key;
          +import org.apache.accumulo.core.data.Value;
          +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
          +import org.apache.accumulo.core.iterators.SortedMapIterator;
          +import org.apache.accumulo.core.iterators.system.ColumnFamilySkippingIterator;
          +import org.apache.accumulo.iteratortest.testcases.IteratorTestCase;
          +
          +/**
          + * A collection of methods that are helpful to the development of

          {@link IteratorTestCase}

          s.
          + */
          +public class IteratorTestUtil {
          +
          + public static SortedKeyValueIterator<Key,Value> instantiateIterator(IteratorTestInput input) {
          + try

          { + return Objects.requireNonNull(input.getIteratorClass()).newInstance(); + }

          catch (InstantiationException | IllegalAccessException e)

          { + throw new RuntimeException(e); + }

          + }
          +
          + public static SortedKeyValueIterator<Key,Value> createSource(IteratorTestInput input) {
          + return new ColumnFamilySkippingIterator(new SortedMapIterator(Objects.requireNonNull(input).getInput()));
          — End diff –

          > Storing the output of a source in a list or map without making a copy is a common pitfall.

          Ooo, that's a good one I didn't think to test. Sounds like a good idea to implicitly test. I can also see a test that checks that Keys and values aren't reused (with a reference check).

          Show
          githubbot ASF GitHub Bot added a comment - Github user joshelser commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r44316590 — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/IteratorTestUtil.java — @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.iteratortest; + +import java.util.Objects; + +import org.apache.accumulo.core.data.Key; +import org.apache.accumulo.core.data.Value; +import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +import org.apache.accumulo.core.iterators.SortedMapIterator; +import org.apache.accumulo.core.iterators.system.ColumnFamilySkippingIterator; +import org.apache.accumulo.iteratortest.testcases.IteratorTestCase; + +/** + * A collection of methods that are helpful to the development of {@link IteratorTestCase} s. + */ +public class IteratorTestUtil { + + public static SortedKeyValueIterator<Key,Value> instantiateIterator(IteratorTestInput input) { + try { + return Objects.requireNonNull(input.getIteratorClass()).newInstance(); + } catch (InstantiationException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } + + public static SortedKeyValueIterator<Key,Value> createSource(IteratorTestInput input) { + return new ColumnFamilySkippingIterator(new SortedMapIterator(Objects.requireNonNull(input).getInput())); — End diff – > Storing the output of a source in a list or map without making a copy is a common pitfall. Ooo, that's a good one I didn't think to test. Sounds like a good idea to implicitly test. I can also see a test that checks that Keys and values aren't reused (with a reference check).
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user keith-turner commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r44318310

          — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/testcases/ReSeekTestCase.java —
          @@ -0,0 +1,107 @@
          +/*
          + * Licensed to the Apache Software Foundation (ASF) under one or more
          + * contributor license agreements. See the NOTICE file distributed with
          + * this work for additional information regarding copyright ownership.
          + * The ASF licenses this file to you under the Apache License, Version 2.0
          + * (the "License"); you may not use this file except in compliance with
          + * the License. You may obtain a copy of the License at
          + *
          + * http://www.apache.org/licenses/LICENSE-2.0
          + *
          + * Unless required by applicable law or agreed to in writing, software
          + * distributed under the License is distributed on an "AS IS" BASIS,
          + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + * See the License for the specific language governing permissions and
          + * limitations under the License.
          + */
          +package org.apache.accumulo.iteratortest.testcases;
          +
          +import java.io.IOException;
          +import java.util.Collections;
          +import java.util.Random;
          +import java.util.TreeMap;
          +
          +import org.apache.accumulo.core.data.ByteSequence;
          +import org.apache.accumulo.core.data.Key;
          +import org.apache.accumulo.core.data.Range;
          +import org.apache.accumulo.core.data.Value;
          +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
          +import org.apache.accumulo.iteratortest.IteratorTestInput;
          +import org.apache.accumulo.iteratortest.IteratorTestOutput;
          +import org.apache.accumulo.iteratortest.IteratorTestUtil;
          +import org.apache.accumulo.iteratortest.environments.SimpleIteratorEnvironment;
          +import org.slf4j.Logger;
          +import org.slf4j.LoggerFactory;
          +
          +/**
          + * Test case that verifies that an iterator can use the generated instance from

          {@code deepCopy}

          .
          + */
          +public class ReSeekTestCase extends OutputVerifyingTestCase {
          + private static final Logger log = LoggerFactory.getLogger(ReSeekTestCase.class);
          +
          + /**
          + * Let N be a random number between [0, RESEEK_INTERVAL). After every Nth entry "returned" to the client, recreate and reseek the iterator.
          + */
          + private static final int RESEEK_INTERVAL = 4;
          +
          + private final Random random;
          +
          + public ReSeekTestCase()

          { + this.random = new Random(); + }

          +
          + @Override
          + public IteratorTestOutput test(IteratorTestInput testInput) {
          + final SortedKeyValueIterator<Key,Value> skvi = IteratorTestUtil.instantiateIterator(testInput);
          + final SortedKeyValueIterator<Key,Value> source = IteratorTestUtil.createSource(testInput);
          +
          + try

          { + skvi.init(source, testInput.getIteratorOptions(), new SimpleIteratorEnvironment()); + skvi.seek(testInput.getRange(), Collections.<ByteSequence> emptySet(), false); + return new IteratorTestOutput(consume(skvi, testInput)); + }

          catch (IOException e)

          { + return new IteratorTestOutput(e); + }

          + }
          +
          + TreeMap<Key,Value> consume(SortedKeyValueIterator<Key,Value> skvi, IteratorTestInput testInput) throws IOException {
          + final TreeMap<Key,Value> data = new TreeMap<>();
          + final Range origRange = testInput.getRange();
          + int reseekCount = random.nextInt(RESEEK_INTERVAL);
          +
          + int i = 0;
          + while (skvi.hasTop()) {
          + data.put(skvi.getTopKey(), skvi.getTopValue());
          +
          + /*
          + * One of the trickiest cases in writing iterators:
          + *
          + * After any result is returned from a TabletServer to the client, the Iterator in the TabletServer's memory may be torn down. To preserve the state and
          + * guarantee that all records are received, the TabletServer does remember the last Key it returned to the client. It will recreate the Iterator (stack),
          + * and seek it using an updated Range. This range's start key is set to the last Key returned, non-inclusive.
          + */
          + if (i % RESEEK_INTERVAL == reseekCount) {
          + // Last key
          + Key reSeekStartKey = skvi.getTopKey();
          +
          + // Deepcopy the iterator
          + skvi = skvi.deepCopy(new SimpleIteratorEnvironment());
          — End diff –

          creating a completely new iterator instance and initializing it would be more realistic

          Show
          githubbot ASF GitHub Bot added a comment - Github user keith-turner commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r44318310 — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/testcases/ReSeekTestCase.java — @@ -0,0 +1,107 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.iteratortest.testcases; + +import java.io.IOException; +import java.util.Collections; +import java.util.Random; +import java.util.TreeMap; + +import org.apache.accumulo.core.data.ByteSequence; +import org.apache.accumulo.core.data.Key; +import org.apache.accumulo.core.data.Range; +import org.apache.accumulo.core.data.Value; +import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +import org.apache.accumulo.iteratortest.IteratorTestInput; +import org.apache.accumulo.iteratortest.IteratorTestOutput; +import org.apache.accumulo.iteratortest.IteratorTestUtil; +import org.apache.accumulo.iteratortest.environments.SimpleIteratorEnvironment; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Test case that verifies that an iterator can use the generated instance from {@code deepCopy} . + */ +public class ReSeekTestCase extends OutputVerifyingTestCase { + private static final Logger log = LoggerFactory.getLogger(ReSeekTestCase.class); + + /** + * Let N be a random number between [0, RESEEK_INTERVAL). After every Nth entry "returned" to the client, recreate and reseek the iterator. + */ + private static final int RESEEK_INTERVAL = 4; + + private final Random random; + + public ReSeekTestCase() { + this.random = new Random(); + } + + @Override + public IteratorTestOutput test(IteratorTestInput testInput) { + final SortedKeyValueIterator<Key,Value> skvi = IteratorTestUtil.instantiateIterator(testInput); + final SortedKeyValueIterator<Key,Value> source = IteratorTestUtil.createSource(testInput); + + try { + skvi.init(source, testInput.getIteratorOptions(), new SimpleIteratorEnvironment()); + skvi.seek(testInput.getRange(), Collections.<ByteSequence> emptySet(), false); + return new IteratorTestOutput(consume(skvi, testInput)); + } catch (IOException e) { + return new IteratorTestOutput(e); + } + } + + TreeMap<Key,Value> consume(SortedKeyValueIterator<Key,Value> skvi, IteratorTestInput testInput) throws IOException { + final TreeMap<Key,Value> data = new TreeMap<>(); + final Range origRange = testInput.getRange(); + int reseekCount = random.nextInt(RESEEK_INTERVAL); + + int i = 0; + while (skvi.hasTop()) { + data.put(skvi.getTopKey(), skvi.getTopValue()); + + /* + * One of the trickiest cases in writing iterators: + * + * After any result is returned from a TabletServer to the client, the Iterator in the TabletServer's memory may be torn down. To preserve the state and + * guarantee that all records are received, the TabletServer does remember the last Key it returned to the client. It will recreate the Iterator (stack), + * and seek it using an updated Range. This range's start key is set to the last Key returned, non-inclusive. + */ + if (i % RESEEK_INTERVAL == reseekCount) { + // Last key + Key reSeekStartKey = skvi.getTopKey(); + + // Deepcopy the iterator + skvi = skvi.deepCopy(new SimpleIteratorEnvironment()); — End diff – creating a completely new iterator instance and initializing it would be more realistic
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user keith-turner commented on the pull request:

          https://github.com/apache/accumulo/pull/50#issuecomment-155165730

          There may be value in running some of these test against an iterator stack and not just a single iterator.

          Show
          githubbot ASF GitHub Bot added a comment - Github user keith-turner commented on the pull request: https://github.com/apache/accumulo/pull/50#issuecomment-155165730 There may be value in running some of these test against an iterator stack and not just a single iterator.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user keith-turner commented on the pull request:

          https://github.com/apache/accumulo/pull/50#issuecomment-155167375

          For iterators that can be run at major compaction, how can we exercise delete keys?

          Show
          githubbot ASF GitHub Bot added a comment - Github user keith-turner commented on the pull request: https://github.com/apache/accumulo/pull/50#issuecomment-155167375 For iterators that can be run at major compaction, how can we exercise delete keys?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user joshelser commented on the pull request:

          https://github.com/apache/accumulo/pull/50#issuecomment-155168971

          > For iterators that can be run at major compaction, how can we exercise delete keys?

          Very good question! The completely mocked-out IteratorEnvironment is a sign that I haven't thought past the scan case. I'm not even sure if the ColumnFiltering iterator with SortedMapIterator is even a 100% valid in-memory reproduction of how it works in a TabletServer.

          I had left input entirely up to the user, so I'm not sure of a simple way to test deletions without potentially messing up some expected table structure. There are some tricky edge cases at compaction that I haven't yet considered how to encapsulate. Combiners on non-full compactions are another good example.

          Show
          githubbot ASF GitHub Bot added a comment - Github user joshelser commented on the pull request: https://github.com/apache/accumulo/pull/50#issuecomment-155168971 > For iterators that can be run at major compaction, how can we exercise delete keys? Very good question! The completely mocked-out IteratorEnvironment is a sign that I haven't thought past the scan case. I'm not even sure if the ColumnFiltering iterator with SortedMapIterator is even a 100% valid in-memory reproduction of how it works in a TabletServer. I had left input entirely up to the user, so I'm not sure of a simple way to test deletions without potentially messing up some expected table structure. There are some tricky edge cases at compaction that I haven't yet considered how to encapsulate. Combiners on non-full compactions are another good example.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user keith-turner commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r44322387

          — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/testcases/DeepCopyTestCase.java —
          @@ -0,0 +1,63 @@
          +/*
          + * Licensed to the Apache Software Foundation (ASF) under one or more
          + * contributor license agreements. See the NOTICE file distributed with
          + * this work for additional information regarding copyright ownership.
          + * The ASF licenses this file to you under the Apache License, Version 2.0
          + * (the "License"); you may not use this file except in compliance with
          + * the License. You may obtain a copy of the License at
          + *
          + * http://www.apache.org/licenses/LICENSE-2.0
          + *
          + * Unless required by applicable law or agreed to in writing, software
          + * distributed under the License is distributed on an "AS IS" BASIS,
          + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + * See the License for the specific language governing permissions and
          + * limitations under the License.
          + */
          +package org.apache.accumulo.iteratortest.testcases;
          +
          +import java.io.IOException;
          +import java.util.Collections;
          +import java.util.TreeMap;
          +
          +import org.apache.accumulo.core.data.ByteSequence;
          +import org.apache.accumulo.core.data.Key;
          +import org.apache.accumulo.core.data.Value;
          +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
          +import org.apache.accumulo.iteratortest.IteratorTestInput;
          +import org.apache.accumulo.iteratortest.IteratorTestOutput;
          +import org.apache.accumulo.iteratortest.IteratorTestUtil;
          +import org.apache.accumulo.iteratortest.environments.SimpleIteratorEnvironment;
          +
          +/**
          + * Test case that verifies that an iterator can use the generated instance from

          {@code deepCopy}

          .
          + */
          +public class DeepCopyTestCase extends OutputVerifyingTestCase {
          +
          + @Override
          + public IteratorTestOutput test(IteratorTestInput testInput) {
          + final SortedKeyValueIterator<Key,Value> skvi = IteratorTestUtil.instantiateIterator(testInput);
          + final SortedKeyValueIterator<Key,Value> source = IteratorTestUtil.createSource(testInput);
          +
          + try {
          + skvi.init(source, testInput.getIteratorOptions(), new SimpleIteratorEnvironment());
          +
          + SortedKeyValueIterator<Key,Value> copy = skvi.deepCopy(new SimpleIteratorEnvironment());
          +
          + copy.seek(testInput.getRange(), Collections.<ByteSequence> emptySet(), false);
          — End diff –

          Seeking one deep copy should have no impact on other deep copies. Would be nice to verify this. Could interleave reading and seeking a few deep copies.

          Show
          githubbot ASF GitHub Bot added a comment - Github user keith-turner commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r44322387 — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/testcases/DeepCopyTestCase.java — @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.iteratortest.testcases; + +import java.io.IOException; +import java.util.Collections; +import java.util.TreeMap; + +import org.apache.accumulo.core.data.ByteSequence; +import org.apache.accumulo.core.data.Key; +import org.apache.accumulo.core.data.Value; +import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +import org.apache.accumulo.iteratortest.IteratorTestInput; +import org.apache.accumulo.iteratortest.IteratorTestOutput; +import org.apache.accumulo.iteratortest.IteratorTestUtil; +import org.apache.accumulo.iteratortest.environments.SimpleIteratorEnvironment; + +/** + * Test case that verifies that an iterator can use the generated instance from {@code deepCopy} . + */ +public class DeepCopyTestCase extends OutputVerifyingTestCase { + + @Override + public IteratorTestOutput test(IteratorTestInput testInput) { + final SortedKeyValueIterator<Key,Value> skvi = IteratorTestUtil.instantiateIterator(testInput); + final SortedKeyValueIterator<Key,Value> source = IteratorTestUtil.createSource(testInput); + + try { + skvi.init(source, testInput.getIteratorOptions(), new SimpleIteratorEnvironment()); + + SortedKeyValueIterator<Key,Value> copy = skvi.deepCopy(new SimpleIteratorEnvironment()); + + copy.seek(testInput.getRange(), Collections.<ByteSequence> emptySet(), false); — End diff – Seeking one deep copy should have no impact on other deep copies. Would be nice to verify this. Could interleave reading and seeking a few deep copies.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ctubbsii commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r44330310

          — Diff: iterator-test-harness/pom.xml —
          @@ -0,0 +1,56 @@
          +<?xml version="1.0" encoding="UTF-8"?>
          +<!--
          + Licensed to the Apache Software Foundation (ASF) under one or more
          + contributor license agreements. See the NOTICE file distributed with
          + this work for additional information regarding copyright ownership.
          + The ASF licenses this file to You under the Apache License, Version 2.0
          + (the "License"); you may not use this file except in compliance with
          + the License. You may obtain a copy of the License at
          +
          + http://www.apache.org/licenses/LICENSE-2.0
          +
          + Unless required by applicable law or agreed to in writing, software
          + distributed under the License is distributed on an "AS IS" BASIS,
          + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + See the License for the specific language governing permissions and
          + limitations under the License.
          +-->
          +<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
          + <modelVersion>4.0.0</modelVersion>
          + <parent>
          + <groupId>org.apache.accumulo</groupId>
          + <artifactId>accumulo-project</artifactId>
          + <version>1.8.0-SNAPSHOT</version>
          + </parent>
          + <artifactId>accumulo-iterator-test-harness</artifactId>
          + <name>Apache Accumulo Iterator Test Harness</name>
          + <description>A library for testing Apache Accumulo Iterators.</description>
          + <dependencies>
          + <!-- TODO Don't force downstream users to have JUnit -->
          + <dependency>
          + <groupId>junit</groupId>
          + <artifactId>junit</artifactId>
          + <!-optional>true</optional->
          + </dependency>
          + <dependency>
          + <groupId>log4j</groupId>
          + <artifactId>log4j</artifactId>
          + </dependency>
          + <dependency>
          + <groupId>org.apache.accumulo</groupId>
          + <artifactId>accumulo-core</artifactId>
          + </dependency>
          + <dependency>
          + <groupId>org.apache.hadoop</groupId>
          + <artifactId>hadoop-client</artifactId>
          + </dependency>
          + <dependency>
          + <groupId>org.reflections</groupId>
          + <artifactId>reflections</artifactId>
          + </dependency>
          — End diff –

          LICENSE/NOTICE include things that we distribute, not things which satisfy our dependencies. So, no, I don't think we need to include anything extra there, unless we're including it's source in our project, or expecting to bundle its jar in our -bin.tar.gz

          As for the WTFPL, it's fine... I remember looking into this a bit for ACCUMULO-1496(https://issues.apache.org/jira/browse/ACCUMULO-1496). That license even permits the code to be freely re-licensed, so even if we did include it, we could include it under the Apache License instead anyway.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ctubbsii commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r44330310 — Diff: iterator-test-harness/pom.xml — @@ -0,0 +1,56 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd "> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>org.apache.accumulo</groupId> + <artifactId>accumulo-project</artifactId> + <version>1.8.0-SNAPSHOT</version> + </parent> + <artifactId>accumulo-iterator-test-harness</artifactId> + <name>Apache Accumulo Iterator Test Harness</name> + <description>A library for testing Apache Accumulo Iterators.</description> + <dependencies> + <!-- TODO Don't force downstream users to have JUnit --> + <dependency> + <groupId>junit</groupId> + <artifactId>junit</artifactId> + <!- optional>true</optional -> + </dependency> + <dependency> + <groupId>log4j</groupId> + <artifactId>log4j</artifactId> + </dependency> + <dependency> + <groupId>org.apache.accumulo</groupId> + <artifactId>accumulo-core</artifactId> + </dependency> + <dependency> + <groupId>org.apache.hadoop</groupId> + <artifactId>hadoop-client</artifactId> + </dependency> + <dependency> + <groupId>org.reflections</groupId> + <artifactId>reflections</artifactId> + </dependency> — End diff – LICENSE/NOTICE include things that we distribute, not things which satisfy our dependencies. So, no, I don't think we need to include anything extra there, unless we're including it's source in our project, or expecting to bundle its jar in our -bin.tar.gz As for the WTFPL, it's fine... I remember looking into this a bit for ACCUMULO-1496 ( https://issues.apache.org/jira/browse/ACCUMULO-1496 ). That license even permits the code to be freely re-licensed, so even if we did include it, we could include it under the Apache License instead anyway.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user madrob commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r44370042

          — Diff: iterator-test-harness/pom.xml —
          @@ -0,0 +1,56 @@
          +<?xml version="1.0" encoding="UTF-8"?>
          +<!--
          + Licensed to the Apache Software Foundation (ASF) under one or more
          + contributor license agreements. See the NOTICE file distributed with
          + this work for additional information regarding copyright ownership.
          + The ASF licenses this file to You under the Apache License, Version 2.0
          + (the "License"); you may not use this file except in compliance with
          + the License. You may obtain a copy of the License at
          +
          + http://www.apache.org/licenses/LICENSE-2.0
          +
          + Unless required by applicable law or agreed to in writing, software
          + distributed under the License is distributed on an "AS IS" BASIS,
          + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + See the License for the specific language governing permissions and
          + limitations under the License.
          +-->
          +<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
          + <modelVersion>4.0.0</modelVersion>
          + <parent>
          + <groupId>org.apache.accumulo</groupId>
          + <artifactId>accumulo-project</artifactId>
          + <version>1.8.0-SNAPSHOT</version>
          + </parent>
          + <artifactId>accumulo-iterator-test-harness</artifactId>
          + <name>Apache Accumulo Iterator Test Harness</name>
          + <description>A library for testing Apache Accumulo Iterators.</description>
          + <dependencies>
          + <!-- TODO Don't force downstream users to have JUnit -->
          + <dependency>
          + <groupId>junit</groupId>
          + <artifactId>junit</artifactId>
          + <!-optional>true</optional->
          + </dependency>
          + <dependency>
          + <groupId>log4j</groupId>
          + <artifactId>log4j</artifactId>
          + </dependency>
          + <dependency>
          + <groupId>org.apache.accumulo</groupId>
          + <artifactId>accumulo-core</artifactId>
          + </dependency>
          + <dependency>
          + <groupId>org.apache.hadoop</groupId>
          + <artifactId>hadoop-client</artifactId>
          + </dependency>
          + <dependency>
          + <groupId>org.reflections</groupId>
          + <artifactId>reflections</artifactId>
          + </dependency>
          — End diff –

          > That license even permits the code to be freely re-licensed, so even if we did include it, we could include it under the Apache License instead anyway.

          This came up in LEGAL-135 and the community felt it was better to leave as is, instead of re-licensing.

          Show
          githubbot ASF GitHub Bot added a comment - Github user madrob commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r44370042 — Diff: iterator-test-harness/pom.xml — @@ -0,0 +1,56 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> +<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd "> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>org.apache.accumulo</groupId> + <artifactId>accumulo-project</artifactId> + <version>1.8.0-SNAPSHOT</version> + </parent> + <artifactId>accumulo-iterator-test-harness</artifactId> + <name>Apache Accumulo Iterator Test Harness</name> + <description>A library for testing Apache Accumulo Iterators.</description> + <dependencies> + <!-- TODO Don't force downstream users to have JUnit --> + <dependency> + <groupId>junit</groupId> + <artifactId>junit</artifactId> + <!- optional>true</optional -> + </dependency> + <dependency> + <groupId>log4j</groupId> + <artifactId>log4j</artifactId> + </dependency> + <dependency> + <groupId>org.apache.accumulo</groupId> + <artifactId>accumulo-core</artifactId> + </dependency> + <dependency> + <groupId>org.apache.hadoop</groupId> + <artifactId>hadoop-client</artifactId> + </dependency> + <dependency> + <groupId>org.reflections</groupId> + <artifactId>reflections</artifactId> + </dependency> — End diff – > That license even permits the code to be freely re-licensed, so even if we did include it, we could include it under the Apache License instead anyway. This came up in LEGAL-135 and the community felt it was better to leave as is, instead of re-licensing.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user joshelser commented on the pull request:

          https://github.com/apache/accumulo/pull/50#issuecomment-163484617

          Rebased on top of master and addressed the current comments:

          • Removed Reflections dependency
          • Reuse Key/Value in the test iterator stack
          • New deepCopy test that iterates over multiple copies.
          • More accurate re-seek test case.

          Only outstanding thing I want to do before an initial commit is to run all of the iterators in o.a.a.c.iterators through this framework in accumulo-test (start the dog-fooding).

          Keith had some suggestions about fancier testing (iterator stacks, compaction/scopes, and delete keys), which I'm not sure I want to try to solve right away. I feel like I've let these changes sit for far too long already. I'd be happy to wait if anyone else is interested in working on it short-term, though.

          Show
          githubbot ASF GitHub Bot added a comment - Github user joshelser commented on the pull request: https://github.com/apache/accumulo/pull/50#issuecomment-163484617 Rebased on top of master and addressed the current comments: Removed Reflections dependency Reuse Key/Value in the test iterator stack New deepCopy test that iterates over multiple copies. More accurate re-seek test case. Only outstanding thing I want to do before an initial commit is to run all of the iterators in o.a.a.c.iterators through this framework in accumulo-test (start the dog-fooding). Keith had some suggestions about fancier testing (iterator stacks, compaction/scopes, and delete keys), which I'm not sure I want to try to solve right away. I feel like I've let these changes sit for far too long already. I'd be happy to wait if anyone else is interested in working on it short-term, though.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user joshelser commented on the pull request:

          https://github.com/apache/accumulo/pull/50#issuecomment-163673419

          Drat, another thing I just thought of: this is meant to be user facing, and I think it should eventually hit public API. It might be premature to put it there immediately (since this hasn't been vetted by anyone but myself), but does anyone have thoughts on a path to "graduate" into public API status?

          Show
          githubbot ASF GitHub Bot added a comment - Github user joshelser commented on the pull request: https://github.com/apache/accumulo/pull/50#issuecomment-163673419 Drat, another thing I just thought of: this is meant to be user facing, and I think it should eventually hit public API. It might be premature to put it there immediately (since this hasn't been vetted by anyone but myself), but does anyone have thoughts on a path to "graduate" into public API status?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user dhutchis commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r47254752

          — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/testcases/DeepCopyTestCase.java —
          @@ -0,0 +1,63 @@
          +/*
          + * Licensed to the Apache Software Foundation (ASF) under one or more
          + * contributor license agreements. See the NOTICE file distributed with
          + * this work for additional information regarding copyright ownership.
          + * The ASF licenses this file to you under the Apache License, Version 2.0
          + * (the "License"); you may not use this file except in compliance with
          + * the License. You may obtain a copy of the License at
          + *
          + * http://www.apache.org/licenses/LICENSE-2.0
          + *
          + * Unless required by applicable law or agreed to in writing, software
          + * distributed under the License is distributed on an "AS IS" BASIS,
          + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + * See the License for the specific language governing permissions and
          + * limitations under the License.
          + */
          +package org.apache.accumulo.iteratortest.testcases;
          +
          +import java.io.IOException;
          +import java.util.Collections;
          +import java.util.TreeMap;
          +
          +import org.apache.accumulo.core.data.ByteSequence;
          +import org.apache.accumulo.core.data.Key;
          +import org.apache.accumulo.core.data.Value;
          +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
          +import org.apache.accumulo.iteratortest.IteratorTestInput;
          +import org.apache.accumulo.iteratortest.IteratorTestOutput;
          +import org.apache.accumulo.iteratortest.IteratorTestUtil;
          +import org.apache.accumulo.iteratortest.environments.SimpleIteratorEnvironment;
          +
          +/**
          + * Test case that verifies that an iterator can use the generated instance from

          {@code deepCopy}

          .
          + */
          +public class DeepCopyTestCase extends OutputVerifyingTestCase {
          +
          + @Override
          + public IteratorTestOutput test(IteratorTestInput testInput) {
          + final SortedKeyValueIterator<Key,Value> skvi = IteratorTestUtil.instantiateIterator(testInput);
          + final SortedKeyValueIterator<Key,Value> source = IteratorTestUtil.createSource(testInput);
          +
          + try {
          + skvi.init(source, testInput.getIteratorOptions(), new SimpleIteratorEnvironment());
          +
          + SortedKeyValueIterator<Key,Value> copy = skvi.deepCopy(new SimpleIteratorEnvironment());
          — End diff –

          Some thoughts on another test-- call deepCopy in the middle of an iteration, in addition to the beginning after init() but before any next() calls. Some iterators may do this in order to "save" the iteration at a certain point.

          Show
          githubbot ASF GitHub Bot added a comment - Github user dhutchis commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r47254752 — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/testcases/DeepCopyTestCase.java — @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.iteratortest.testcases; + +import java.io.IOException; +import java.util.Collections; +import java.util.TreeMap; + +import org.apache.accumulo.core.data.ByteSequence; +import org.apache.accumulo.core.data.Key; +import org.apache.accumulo.core.data.Value; +import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +import org.apache.accumulo.iteratortest.IteratorTestInput; +import org.apache.accumulo.iteratortest.IteratorTestOutput; +import org.apache.accumulo.iteratortest.IteratorTestUtil; +import org.apache.accumulo.iteratortest.environments.SimpleIteratorEnvironment; + +/** + * Test case that verifies that an iterator can use the generated instance from {@code deepCopy} . + */ +public class DeepCopyTestCase extends OutputVerifyingTestCase { + + @Override + public IteratorTestOutput test(IteratorTestInput testInput) { + final SortedKeyValueIterator<Key,Value> skvi = IteratorTestUtil.instantiateIterator(testInput); + final SortedKeyValueIterator<Key,Value> source = IteratorTestUtil.createSource(testInput); + + try { + skvi.init(source, testInput.getIteratorOptions(), new SimpleIteratorEnvironment()); + + SortedKeyValueIterator<Key,Value> copy = skvi.deepCopy(new SimpleIteratorEnvironment()); — End diff – Some thoughts on another test-- call deepCopy in the middle of an iteration, in addition to the beginning after init() but before any next() calls. Some iterators may do this in order to "save" the iteration at a certain point.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user joshelser commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r47255171

          — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/testcases/DeepCopyTestCase.java —
          @@ -0,0 +1,63 @@
          +/*
          + * Licensed to the Apache Software Foundation (ASF) under one or more
          + * contributor license agreements. See the NOTICE file distributed with
          + * this work for additional information regarding copyright ownership.
          + * The ASF licenses this file to you under the Apache License, Version 2.0
          + * (the "License"); you may not use this file except in compliance with
          + * the License. You may obtain a copy of the License at
          + *
          + * http://www.apache.org/licenses/LICENSE-2.0
          + *
          + * Unless required by applicable law or agreed to in writing, software
          + * distributed under the License is distributed on an "AS IS" BASIS,
          + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + * See the License for the specific language governing permissions and
          + * limitations under the License.
          + */
          +package org.apache.accumulo.iteratortest.testcases;
          +
          +import java.io.IOException;
          +import java.util.Collections;
          +import java.util.TreeMap;
          +
          +import org.apache.accumulo.core.data.ByteSequence;
          +import org.apache.accumulo.core.data.Key;
          +import org.apache.accumulo.core.data.Value;
          +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
          +import org.apache.accumulo.iteratortest.IteratorTestInput;
          +import org.apache.accumulo.iteratortest.IteratorTestOutput;
          +import org.apache.accumulo.iteratortest.IteratorTestUtil;
          +import org.apache.accumulo.iteratortest.environments.SimpleIteratorEnvironment;
          +
          +/**
          + * Test case that verifies that an iterator can use the generated instance from

          {@code deepCopy}

          .
          + */
          +public class DeepCopyTestCase extends OutputVerifyingTestCase {
          +
          + @Override
          + public IteratorTestOutput test(IteratorTestInput testInput) {
          + final SortedKeyValueIterator<Key,Value> skvi = IteratorTestUtil.instantiateIterator(testInput);
          + final SortedKeyValueIterator<Key,Value> source = IteratorTestUtil.createSource(testInput);
          +
          + try {
          + skvi.init(source, testInput.getIteratorOptions(), new SimpleIteratorEnvironment());
          +
          + SortedKeyValueIterator<Key,Value> copy = skvi.deepCopy(new SimpleIteratorEnvironment());
          — End diff –

          Thanks for taking a look, Dylan!

          > call deepCopy in the middle of an iteration, in addition to the beginning after init() but before any next() calls

          Yeah, there are lots of good tests we can do around deepCopy. I'll see if I can add one for this specific case. Please feel free to send a PR if you definitely want to see this in the initial changeset

          > Some iterators may do this in order to "save" the iteration at a certain point.

          But that's not something the "system" does, is it? I can see the value, but I'm not sure if that's something that would happen through "normal" Accumulo use.

          Show
          githubbot ASF GitHub Bot added a comment - Github user joshelser commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r47255171 — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/testcases/DeepCopyTestCase.java — @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.iteratortest.testcases; + +import java.io.IOException; +import java.util.Collections; +import java.util.TreeMap; + +import org.apache.accumulo.core.data.ByteSequence; +import org.apache.accumulo.core.data.Key; +import org.apache.accumulo.core.data.Value; +import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +import org.apache.accumulo.iteratortest.IteratorTestInput; +import org.apache.accumulo.iteratortest.IteratorTestOutput; +import org.apache.accumulo.iteratortest.IteratorTestUtil; +import org.apache.accumulo.iteratortest.environments.SimpleIteratorEnvironment; + +/** + * Test case that verifies that an iterator can use the generated instance from {@code deepCopy} . + */ +public class DeepCopyTestCase extends OutputVerifyingTestCase { + + @Override + public IteratorTestOutput test(IteratorTestInput testInput) { + final SortedKeyValueIterator<Key,Value> skvi = IteratorTestUtil.instantiateIterator(testInput); + final SortedKeyValueIterator<Key,Value> source = IteratorTestUtil.createSource(testInput); + + try { + skvi.init(source, testInput.getIteratorOptions(), new SimpleIteratorEnvironment()); + + SortedKeyValueIterator<Key,Value> copy = skvi.deepCopy(new SimpleIteratorEnvironment()); — End diff – Thanks for taking a look, Dylan! > call deepCopy in the middle of an iteration, in addition to the beginning after init() but before any next() calls Yeah, there are lots of good tests we can do around deepCopy. I'll see if I can add one for this specific case. Please feel free to send a PR if you definitely want to see this in the initial changeset > Some iterators may do this in order to "save" the iteration at a certain point. But that's not something the "system" does, is it? I can see the value, but I'm not sure if that's something that would happen through "normal" Accumulo use.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user dhutchis commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r47256202

          — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/testcases/DeepCopyTestCase.java —
          @@ -0,0 +1,63 @@
          +/*
          + * Licensed to the Apache Software Foundation (ASF) under one or more
          + * contributor license agreements. See the NOTICE file distributed with
          + * this work for additional information regarding copyright ownership.
          + * The ASF licenses this file to you under the Apache License, Version 2.0
          + * (the "License"); you may not use this file except in compliance with
          + * the License. You may obtain a copy of the License at
          + *
          + * http://www.apache.org/licenses/LICENSE-2.0
          + *
          + * Unless required by applicable law or agreed to in writing, software
          + * distributed under the License is distributed on an "AS IS" BASIS,
          + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + * See the License for the specific language governing permissions and
          + * limitations under the License.
          + */
          +package org.apache.accumulo.iteratortest.testcases;
          +
          +import java.io.IOException;
          +import java.util.Collections;
          +import java.util.TreeMap;
          +
          +import org.apache.accumulo.core.data.ByteSequence;
          +import org.apache.accumulo.core.data.Key;
          +import org.apache.accumulo.core.data.Value;
          +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
          +import org.apache.accumulo.iteratortest.IteratorTestInput;
          +import org.apache.accumulo.iteratortest.IteratorTestOutput;
          +import org.apache.accumulo.iteratortest.IteratorTestUtil;
          +import org.apache.accumulo.iteratortest.environments.SimpleIteratorEnvironment;
          +
          +/**
          + * Test case that verifies that an iterator can use the generated instance from

          {@code deepCopy}

          .
          + */
          +public class DeepCopyTestCase extends OutputVerifyingTestCase {
          +
          + @Override
          + public IteratorTestOutput test(IteratorTestInput testInput) {
          + final SortedKeyValueIterator<Key,Value> skvi = IteratorTestUtil.instantiateIterator(testInput);
          + final SortedKeyValueIterator<Key,Value> source = IteratorTestUtil.createSource(testInput);
          +
          + try {
          + skvi.init(source, testInput.getIteratorOptions(), new SimpleIteratorEnvironment());
          +
          + SortedKeyValueIterator<Key,Value> copy = skvi.deepCopy(new SimpleIteratorEnvironment());
          — End diff –

          Ah I see Josh-- is the purpose to test only system iterators, as opposed to generic tests to check user-defined iterators? That makes more sense.

          Some users may define iterators that expect a column family, whereas all the tests seek with `Collections.<ByteSequence> emptySet(), false`. There may be many other places where a user does something "non-normal" intentionally. If we make this public API, we should state that user iterators do not necessarily need to pass all the IteratorTestCases, or at least that failing an IteratorTestCase does not imply a bug.

          Show
          githubbot ASF GitHub Bot added a comment - Github user dhutchis commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r47256202 — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/testcases/DeepCopyTestCase.java — @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.iteratortest.testcases; + +import java.io.IOException; +import java.util.Collections; +import java.util.TreeMap; + +import org.apache.accumulo.core.data.ByteSequence; +import org.apache.accumulo.core.data.Key; +import org.apache.accumulo.core.data.Value; +import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +import org.apache.accumulo.iteratortest.IteratorTestInput; +import org.apache.accumulo.iteratortest.IteratorTestOutput; +import org.apache.accumulo.iteratortest.IteratorTestUtil; +import org.apache.accumulo.iteratortest.environments.SimpleIteratorEnvironment; + +/** + * Test case that verifies that an iterator can use the generated instance from {@code deepCopy} . + */ +public class DeepCopyTestCase extends OutputVerifyingTestCase { + + @Override + public IteratorTestOutput test(IteratorTestInput testInput) { + final SortedKeyValueIterator<Key,Value> skvi = IteratorTestUtil.instantiateIterator(testInput); + final SortedKeyValueIterator<Key,Value> source = IteratorTestUtil.createSource(testInput); + + try { + skvi.init(source, testInput.getIteratorOptions(), new SimpleIteratorEnvironment()); + + SortedKeyValueIterator<Key,Value> copy = skvi.deepCopy(new SimpleIteratorEnvironment()); — End diff – Ah I see Josh-- is the purpose to test only system iterators, as opposed to generic tests to check user-defined iterators? That makes more sense. Some users may define iterators that expect a column family, whereas all the tests seek with `Collections.<ByteSequence> emptySet(), false`. There may be many other places where a user does something "non-normal" intentionally. If we make this public API, we should state that user iterators do not necessarily need to pass all the IteratorTestCases, or at least that failing an IteratorTestCase does not imply a bug.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user dhutchis commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r47257053

          — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/testcases/DeepCopyTestCase.java —
          @@ -0,0 +1,63 @@
          +/*
          + * Licensed to the Apache Software Foundation (ASF) under one or more
          + * contributor license agreements. See the NOTICE file distributed with
          + * this work for additional information regarding copyright ownership.
          + * The ASF licenses this file to you under the Apache License, Version 2.0
          + * (the "License"); you may not use this file except in compliance with
          + * the License. You may obtain a copy of the License at
          + *
          + * http://www.apache.org/licenses/LICENSE-2.0
          + *
          + * Unless required by applicable law or agreed to in writing, software
          + * distributed under the License is distributed on an "AS IS" BASIS,
          + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + * See the License for the specific language governing permissions and
          + * limitations under the License.
          + */
          +package org.apache.accumulo.iteratortest.testcases;
          +
          +import java.io.IOException;
          +import java.util.Collections;
          +import java.util.TreeMap;
          +
          +import org.apache.accumulo.core.data.ByteSequence;
          +import org.apache.accumulo.core.data.Key;
          +import org.apache.accumulo.core.data.Value;
          +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
          +import org.apache.accumulo.iteratortest.IteratorTestInput;
          +import org.apache.accumulo.iteratortest.IteratorTestOutput;
          +import org.apache.accumulo.iteratortest.IteratorTestUtil;
          +import org.apache.accumulo.iteratortest.environments.SimpleIteratorEnvironment;
          +
          +/**
          + * Test case that verifies that an iterator can use the generated instance from

          {@code deepCopy}

          .
          + */
          +public class DeepCopyTestCase extends OutputVerifyingTestCase {
          +
          + @Override
          + public IteratorTestOutput test(IteratorTestInput testInput) {
          + final SortedKeyValueIterator<Key,Value> skvi = IteratorTestUtil.instantiateIterator(testInput);
          + final SortedKeyValueIterator<Key,Value> source = IteratorTestUtil.createSource(testInput);
          +
          + try {
          + skvi.init(source, testInput.getIteratorOptions(), new SimpleIteratorEnvironment());
          +
          + SortedKeyValueIterator<Key,Value> copy = skvi.deepCopy(new SimpleIteratorEnvironment());
          — End diff –

          Further along that note, many user iterators leave deepCopy() as `throw new UnsupportedOperationException()`. They would fail many of these IteratorTestCases.

          Show
          githubbot ASF GitHub Bot added a comment - Github user dhutchis commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r47257053 — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/testcases/DeepCopyTestCase.java — @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.iteratortest.testcases; + +import java.io.IOException; +import java.util.Collections; +import java.util.TreeMap; + +import org.apache.accumulo.core.data.ByteSequence; +import org.apache.accumulo.core.data.Key; +import org.apache.accumulo.core.data.Value; +import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +import org.apache.accumulo.iteratortest.IteratorTestInput; +import org.apache.accumulo.iteratortest.IteratorTestOutput; +import org.apache.accumulo.iteratortest.IteratorTestUtil; +import org.apache.accumulo.iteratortest.environments.SimpleIteratorEnvironment; + +/** + * Test case that verifies that an iterator can use the generated instance from {@code deepCopy} . + */ +public class DeepCopyTestCase extends OutputVerifyingTestCase { + + @Override + public IteratorTestOutput test(IteratorTestInput testInput) { + final SortedKeyValueIterator<Key,Value> skvi = IteratorTestUtil.instantiateIterator(testInput); + final SortedKeyValueIterator<Key,Value> source = IteratorTestUtil.createSource(testInput); + + try { + skvi.init(source, testInput.getIteratorOptions(), new SimpleIteratorEnvironment()); + + SortedKeyValueIterator<Key,Value> copy = skvi.deepCopy(new SimpleIteratorEnvironment()); — End diff – Further along that note, many user iterators leave deepCopy() as `throw new UnsupportedOperationException()`. They would fail many of these IteratorTestCases.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user joshelser commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r47257077

          — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/testcases/DeepCopyTestCase.java —
          @@ -0,0 +1,63 @@
          +/*
          + * Licensed to the Apache Software Foundation (ASF) under one or more
          + * contributor license agreements. See the NOTICE file distributed with
          + * this work for additional information regarding copyright ownership.
          + * The ASF licenses this file to you under the Apache License, Version 2.0
          + * (the "License"); you may not use this file except in compliance with
          + * the License. You may obtain a copy of the License at
          + *
          + * http://www.apache.org/licenses/LICENSE-2.0
          + *
          + * Unless required by applicable law or agreed to in writing, software
          + * distributed under the License is distributed on an "AS IS" BASIS,
          + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + * See the License for the specific language governing permissions and
          + * limitations under the License.
          + */
          +package org.apache.accumulo.iteratortest.testcases;
          +
          +import java.io.IOException;
          +import java.util.Collections;
          +import java.util.TreeMap;
          +
          +import org.apache.accumulo.core.data.ByteSequence;
          +import org.apache.accumulo.core.data.Key;
          +import org.apache.accumulo.core.data.Value;
          +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
          +import org.apache.accumulo.iteratortest.IteratorTestInput;
          +import org.apache.accumulo.iteratortest.IteratorTestOutput;
          +import org.apache.accumulo.iteratortest.IteratorTestUtil;
          +import org.apache.accumulo.iteratortest.environments.SimpleIteratorEnvironment;
          +
          +/**
          + * Test case that verifies that an iterator can use the generated instance from

          {@code deepCopy}

          .
          + */
          +public class DeepCopyTestCase extends OutputVerifyingTestCase {
          +
          + @Override
          + public IteratorTestOutput test(IteratorTestInput testInput) {
          + final SortedKeyValueIterator<Key,Value> skvi = IteratorTestUtil.instantiateIterator(testInput);
          + final SortedKeyValueIterator<Key,Value> source = IteratorTestUtil.createSource(testInput);
          +
          + try {
          + skvi.init(source, testInput.getIteratorOptions(), new SimpleIteratorEnvironment());
          +
          + SortedKeyValueIterator<Key,Value> copy = skvi.deepCopy(new SimpleIteratorEnvironment());
          — End diff –

          > is the purpose to test only system iterators, as opposed to generic tests to check user-defined iterators? That makes more sense.

          Yep. First and foremost, my goal was to provide a framework for users to test iterators without running them through Accumulo (since it's often hard to know when you're hitting some of the edge cases). However, I can totally see it growing into a framework to catch "common pitfalls" like you outlined

          > Some users may define iterators that expect a column family, whereas all the tests seek with Collections.<ByteSequence> emptySet(), false

          Yeah, this is something I'm not quite sure how to handle colfams (I'm throwing that into the same bucket as the compaction stuff and more that Keith pointed out).

          > If we make this public API, we should state that user iterators do not necessarily need to pass all the IteratorTestCases, or at least that failing an IteratorTestCase does not imply a bug.

          Very good point. We don't really have an "evolving" annotation. Maybe we just talk about it and the lack of public API declares it as evolving intrinsically?

          Show
          githubbot ASF GitHub Bot added a comment - Github user joshelser commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r47257077 — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/testcases/DeepCopyTestCase.java — @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.iteratortest.testcases; + +import java.io.IOException; +import java.util.Collections; +import java.util.TreeMap; + +import org.apache.accumulo.core.data.ByteSequence; +import org.apache.accumulo.core.data.Key; +import org.apache.accumulo.core.data.Value; +import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +import org.apache.accumulo.iteratortest.IteratorTestInput; +import org.apache.accumulo.iteratortest.IteratorTestOutput; +import org.apache.accumulo.iteratortest.IteratorTestUtil; +import org.apache.accumulo.iteratortest.environments.SimpleIteratorEnvironment; + +/** + * Test case that verifies that an iterator can use the generated instance from {@code deepCopy} . + */ +public class DeepCopyTestCase extends OutputVerifyingTestCase { + + @Override + public IteratorTestOutput test(IteratorTestInput testInput) { + final SortedKeyValueIterator<Key,Value> skvi = IteratorTestUtil.instantiateIterator(testInput); + final SortedKeyValueIterator<Key,Value> source = IteratorTestUtil.createSource(testInput); + + try { + skvi.init(source, testInput.getIteratorOptions(), new SimpleIteratorEnvironment()); + + SortedKeyValueIterator<Key,Value> copy = skvi.deepCopy(new SimpleIteratorEnvironment()); — End diff – > is the purpose to test only system iterators, as opposed to generic tests to check user-defined iterators? That makes more sense. Yep. First and foremost, my goal was to provide a framework for users to test iterators without running them through Accumulo (since it's often hard to know when you're hitting some of the edge cases). However, I can totally see it growing into a framework to catch "common pitfalls" like you outlined > Some users may define iterators that expect a column family, whereas all the tests seek with Collections.<ByteSequence> emptySet(), false Yeah, this is something I'm not quite sure how to handle colfams (I'm throwing that into the same bucket as the compaction stuff and more that Keith pointed out). > If we make this public API, we should state that user iterators do not necessarily need to pass all the IteratorTestCases, or at least that failing an IteratorTestCase does not imply a bug. Very good point. We don't really have an "evolving" annotation. Maybe we just talk about it and the lack of public API declares it as evolving intrinsically?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user keith-turner commented on the pull request:

          https://github.com/apache/accumulo/pull/50#issuecomment-163715371

          > but does anyone have thoughts on a path to "graduate" into public API status?

          Well currently iterators are not declared as public API.

          Show
          githubbot ASF GitHub Bot added a comment - Github user keith-turner commented on the pull request: https://github.com/apache/accumulo/pull/50#issuecomment-163715371 > but does anyone have thoughts on a path to "graduate" into public API status? Well currently iterators are not declared as public API.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user joshelser commented on the pull request:

          https://github.com/apache/accumulo/pull/50#issuecomment-163742584

          > Well currently iterators are not declared as public API.

          Ooo, that is a good point (which I often forget). Perhaps I'll just table the idea for now and we can address if/when we do something with iterators.

          Show
          githubbot ASF GitHub Bot added a comment - Github user joshelser commented on the pull request: https://github.com/apache/accumulo/pull/50#issuecomment-163742584 > Well currently iterators are not declared as public API. Ooo, that is a good point (which I often forget). Perhaps I'll just table the idea for now and we can address if/when we do something with iterators.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user joshelser commented on the pull request:

          https://github.com/apache/accumulo/pull/50#issuecomment-163845209

          Okie doke. Pulled in Dylan's test improvements (thanks again for that!). I think the only thing I have left that I want to do is to wire up as many of the existing iterators we provide for users with some tests in the accumulo-test module.

          Any concerns, speak up soon. I'll likely merge over the weekend unless I hear objection to not do so. Then, I will file some follow-on JIRA work for the lingering improvements.

          Show
          githubbot ASF GitHub Bot added a comment - Github user joshelser commented on the pull request: https://github.com/apache/accumulo/pull/50#issuecomment-163845209 Okie doke. Pulled in Dylan's test improvements (thanks again for that!). I think the only thing I have left that I want to do is to wire up as many of the existing iterators we provide for users with some tests in the accumulo-test module. Any concerns, speak up soon. I'll likely merge over the weekend unless I hear objection to not do so. Then, I will file some follow-on JIRA work for the lingering improvements.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user joshelser commented on a diff in the pull request:

          https://github.com/apache/accumulo/pull/50#discussion_r47451909

          — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/IteratorTestUtil.java —
          @@ -0,0 +1,44 @@
          +/*
          + * Licensed to the Apache Software Foundation (ASF) under one or more
          + * contributor license agreements. See the NOTICE file distributed with
          + * this work for additional information regarding copyright ownership.
          + * The ASF licenses this file to you under the Apache License, Version 2.0
          + * (the "License"); you may not use this file except in compliance with
          + * the License. You may obtain a copy of the License at
          + *
          + * http://www.apache.org/licenses/LICENSE-2.0
          + *
          + * Unless required by applicable law or agreed to in writing, software
          + * distributed under the License is distributed on an "AS IS" BASIS,
          + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
          + * See the License for the specific language governing permissions and
          + * limitations under the License.
          + */
          +package org.apache.accumulo.iteratortest;
          +
          +import java.util.Objects;
          +
          +import org.apache.accumulo.core.data.Key;
          +import org.apache.accumulo.core.data.Value;
          +import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
          +import org.apache.accumulo.core.iterators.SortedMapIterator;
          +import org.apache.accumulo.core.iterators.system.ColumnFamilySkippingIterator;
          +import org.apache.accumulo.iteratortest.testcases.IteratorTestCase;
          +
          +/**
          + * A collection of methods that are helpful to the development of

          {@link IteratorTestCase}

          s.
          + */
          +public class IteratorTestUtil {
          +
          + public static SortedKeyValueIterator<Key,Value> instantiateIterator(IteratorTestInput input) {
          + try

          { + return Objects.requireNonNull(input.getIteratorClass()).newInstance(); + }

          catch (InstantiationException | IllegalAccessException e)

          { + throw new RuntimeException(e); + }

          + }
          +
          + public static SortedKeyValueIterator<Key,Value> createSource(IteratorTestInput input) {
          + return new ColumnFamilySkippingIterator(new SortedMapIterator(Objects.requireNonNull(input).getInput()));
          — End diff –

          Turns out, this was a really good suggestion. Helped me find a bug in my test cases (need to make sure to copy the Key-Value being emitted). The Tablet Scanner does this when converting the Key and Value from the iterator stack into a `KeyValue`. Thanks again, Bill!

          Show
          githubbot ASF GitHub Bot added a comment - Github user joshelser commented on a diff in the pull request: https://github.com/apache/accumulo/pull/50#discussion_r47451909 — Diff: iterator-test-harness/src/main/java/org/apache/accumulo/iteratortest/IteratorTestUtil.java — @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.accumulo.iteratortest; + +import java.util.Objects; + +import org.apache.accumulo.core.data.Key; +import org.apache.accumulo.core.data.Value; +import org.apache.accumulo.core.iterators.SortedKeyValueIterator; +import org.apache.accumulo.core.iterators.SortedMapIterator; +import org.apache.accumulo.core.iterators.system.ColumnFamilySkippingIterator; +import org.apache.accumulo.iteratortest.testcases.IteratorTestCase; + +/** + * A collection of methods that are helpful to the development of {@link IteratorTestCase} s. + */ +public class IteratorTestUtil { + + public static SortedKeyValueIterator<Key,Value> instantiateIterator(IteratorTestInput input) { + try { + return Objects.requireNonNull(input.getIteratorClass()).newInstance(); + } catch (InstantiationException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } + + public static SortedKeyValueIterator<Key,Value> createSource(IteratorTestInput input) { + return new ColumnFamilySkippingIterator(new SortedMapIterator(Objects.requireNonNull(input).getInput())); — End diff – Turns out, this was a really good suggestion. Helped me find a bug in my test cases (need to make sure to copy the Key-Value being emitted). The Tablet Scanner does this when converting the Key and Value from the iterator stack into a `KeyValue`. Thanks again, Bill!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/accumulo/pull/50

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/accumulo/pull/50
          Hide
          elserj Josh Elser added a comment -

          Merged in the pull request.

          Thanks again to Dylan, Keith and BIll for your suggestions/reviews along the way!

          Show
          elserj Josh Elser added a comment - Merged in the pull request. Thanks again to Dylan, Keith and BIll for your suggestions/reviews along the way!

            People

            • Assignee:
              elserj Josh Elser
              Reporter:
              ecn Eric Newton
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0h
                0h
                Logged:
                Time Spent - 0.5h
                0.5h

                  Development