From 893a44ea26048bb0e1c0679dfd5fda4ececd183d Mon Sep 17 00:00:00 2001 From: Gokulakrishnan <7522696+gokulsfdc@users.noreply.github.com> Date: Mon, 20 Aug 2018 16:08:13 +0530 Subject: [PATCH 1/3] Adding unit test for FeatureHistory --- .../salesforce/op/FeatureHistoryTest.scala | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 utils/src/test/scala/com/salesforce/op/FeatureHistoryTest.scala diff --git a/utils/src/test/scala/com/salesforce/op/FeatureHistoryTest.scala b/utils/src/test/scala/com/salesforce/op/FeatureHistoryTest.scala new file mode 100644 index 0000000000..f858fee6de --- /dev/null +++ b/utils/src/test/scala/com/salesforce/op/FeatureHistoryTest.scala @@ -0,0 +1,115 @@ +/* + * Copyright (c) 2017, Salesforce.com, Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * * Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * + * * Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * * Neither the name of the copyright holder nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package com.salesforce.op + +import com.salesforce.op.FeatureHistory.{OriginFeatureKey, StagesKey} +import com.salesforce.op.test.TestCommon +import org.apache.spark.sql.types.MetadataBuilder +import org.junit.runner.RunWith +import org.scalatest.FlatSpec +import org.scalatest.junit.JUnitRunner + +@RunWith(classOf[JUnitRunner]) +class FeatureHistoryTest extends FlatSpec with TestCommon { + + val feature1 = "feature1" + val feature2 = "feature2" + val stage1 = "stage1" + val stage2 = "stage2" + + Spec(FeatureHistory.getClass) should "convert a feature history to metadata" in { + val featureHistory = FeatureHistory(originFeatures = Seq(feature1, feature2), stages = Seq(stage1, stage2)) + + val featureHistoryMetadata = featureHistory.toMetadata + + featureHistoryMetadata.contains(OriginFeatureKey) shouldBe true + featureHistoryMetadata.contains(StagesKey) shouldBe true + + featureHistoryMetadata.getStringArray(OriginFeatureKey) shouldBe Array(feature1, feature2) + featureHistoryMetadata.getStringArray(StagesKey) shouldBe Array(stage1, stage2) + } + + it should "merge two featurehistory" in { + val featureHistory1 = FeatureHistory(originFeatures = Seq(feature1), stages = Seq(stage1)) + val featureHistory2 = FeatureHistory(originFeatures = Seq(feature2), stages = Seq(stage2)) + + val featureHistoryCombined = featureHistory1.merge(featureHistory2) + featureHistoryCombined.originFeatures shouldBe Seq(feature1, feature2) + featureHistoryCombined.stages shouldBe Seq(stage1, stage2) + } + + it should "create a metadata for a featurehistory map" in { + val featureHistory1 = FeatureHistory(originFeatures = Seq(feature1), stages = Seq(stage1)) + val featureHistory2 = FeatureHistory(originFeatures = Seq(feature2), stages = Seq(stage2)) + + val map = Map[String, FeatureHistory](("1" -> featureHistory1), ("2" -> featureHistory2)) + val featureHistoryMetadata = FeatureHistory.toMetadata(map) + + featureHistoryMetadata.contains("1") shouldBe true + featureHistoryMetadata.contains("2") shouldBe true + + val f1 = featureHistoryMetadata.getMetadata("1") + + f1.contains(OriginFeatureKey) shouldBe true + f1.contains(StagesKey) shouldBe true + + f1.getStringArray(OriginFeatureKey) shouldBe Array(feature1) + f1.getStringArray(StagesKey) shouldBe Array(stage1) + + val f2 = featureHistoryMetadata.getMetadata("2") + + f2.contains(OriginFeatureKey) shouldBe true + f2.contains(StagesKey) shouldBe true + + f2.getStringArray(OriginFeatureKey) shouldBe Array(feature2) + f2.getStringArray(StagesKey) shouldBe Array(stage2) + } + + it should "create a FeatureHistory Map from metadata" in { + + val featureHistory1 = FeatureHistory(originFeatures = Seq(feature1), stages = Seq(stage1)) + val featureHistory2 = FeatureHistory(originFeatures = Seq(feature2), stages = Seq(stage2)) + + val featureHistoryMapMetadata = new MetadataBuilder() + .putMetadata("1", featureHistory1.toMetadata) + .putMetadata("2", featureHistory2.toMetadata) + .build() + + val featureHistoryMap = FeatureHistory.fromMetadataMap(featureHistoryMapMetadata) + + featureHistoryMap.contains("1") shouldBe true + featureHistoryMap("1") shouldBe featureHistory1 + + featureHistoryMap.contains("2") shouldBe true + featureHistoryMap("2") shouldBe featureHistory2 + } +} + From 9f60c656e8fa2ffebf6a12fc5648815f92df228f Mon Sep 17 00:00:00 2001 From: Gokulakrishnan <7522696+gokulsfdc@users.noreply.github.com> Date: Mon, 20 Aug 2018 19:40:27 +0530 Subject: [PATCH 2/3] Incorporating code review comments --- .../test/scala/com/salesforce/op/FeatureHistoryTest.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/utils/src/test/scala/com/salesforce/op/FeatureHistoryTest.scala b/utils/src/test/scala/com/salesforce/op/FeatureHistoryTest.scala index f858fee6de..5a63f65e45 100644 --- a/utils/src/test/scala/com/salesforce/op/FeatureHistoryTest.scala +++ b/utils/src/test/scala/com/salesforce/op/FeatureHistoryTest.scala @@ -45,7 +45,7 @@ class FeatureHistoryTest extends FlatSpec with TestCommon { val stage1 = "stage1" val stage2 = "stage2" - Spec(FeatureHistory.getClass) should "convert a feature history to metadata" in { + Spec[FeatureHistory] should "convert a feature history to metadata" in { val featureHistory = FeatureHistory(originFeatures = Seq(feature1, feature2), stages = Seq(stage1, stage2)) val featureHistoryMetadata = featureHistory.toMetadata @@ -57,7 +57,7 @@ class FeatureHistoryTest extends FlatSpec with TestCommon { featureHistoryMetadata.getStringArray(StagesKey) shouldBe Array(stage1, stage2) } - it should "merge two featurehistory" in { + it should "merge two instances" in { val featureHistory1 = FeatureHistory(originFeatures = Seq(feature1), stages = Seq(stage1)) val featureHistory2 = FeatureHistory(originFeatures = Seq(feature2), stages = Seq(stage2)) @@ -70,7 +70,7 @@ class FeatureHistoryTest extends FlatSpec with TestCommon { val featureHistory1 = FeatureHistory(originFeatures = Seq(feature1), stages = Seq(stage1)) val featureHistory2 = FeatureHistory(originFeatures = Seq(feature2), stages = Seq(stage2)) - val map = Map[String, FeatureHistory](("1" -> featureHistory1), ("2" -> featureHistory2)) + val map = Map(("1" -> featureHistory1), ("2" -> featureHistory2)) val featureHistoryMetadata = FeatureHistory.toMetadata(map) featureHistoryMetadata.contains("1") shouldBe true From 42592d9fe16caf7fffae95eb3118dc6503c3f40d Mon Sep 17 00:00:00 2001 From: Gokulakrishnan <7522696+gokulsfdc@users.noreply.github.com> Date: Mon, 20 Aug 2018 21:47:30 +0530 Subject: [PATCH 3/3] code review comments --- .../src/test/scala/com/salesforce/op/FeatureHistoryTest.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/src/test/scala/com/salesforce/op/FeatureHistoryTest.scala b/utils/src/test/scala/com/salesforce/op/FeatureHistoryTest.scala index 5a63f65e45..64721550a5 100644 --- a/utils/src/test/scala/com/salesforce/op/FeatureHistoryTest.scala +++ b/utils/src/test/scala/com/salesforce/op/FeatureHistoryTest.scala @@ -66,7 +66,7 @@ class FeatureHistoryTest extends FlatSpec with TestCommon { featureHistoryCombined.stages shouldBe Seq(stage1, stage2) } - it should "create a metadata for a featurehistory map" in { + it should "create a metadata for a map" in { val featureHistory1 = FeatureHistory(originFeatures = Seq(feature1), stages = Seq(stage1)) val featureHistory2 = FeatureHistory(originFeatures = Seq(feature2), stages = Seq(stage2)) @@ -93,7 +93,7 @@ class FeatureHistoryTest extends FlatSpec with TestCommon { f2.getStringArray(StagesKey) shouldBe Array(stage2) } - it should "create a FeatureHistory Map from metadata" in { + it should "create a map from metadata" in { val featureHistory1 = FeatureHistory(originFeatures = Seq(feature1), stages = Seq(stage1)) val featureHistory2 = FeatureHistory(originFeatures = Seq(feature2), stages = Seq(stage2))