Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding Array of metadataSeq in RichMap #72

Merged
merged 2 commits into from Aug 20, 2018

Conversation

gokulsfdc
Copy link
Contributor

In RichMap, for Array values, we are currently accepting booleanSeq, intSeq, longSeq, doubleSeq and stringSeq.

  • Adding one more datatype to the list - MetadataSeq.
  • Adding test case for the new type added.
  • Adding test case for withSummary() as well for improved coverage

@@ -85,6 +85,11 @@ class RichMetadataTest extends FlatSpec with TestCommon {
val mergedMetadata = meta1.deepMerge(map2.toMetadata)
mergedMetadata.json shouldBe Serialization.write(mergedMap)

val m1 = Map("1" -> Array(Map("val" -> "a").toMetadata)).toMetadata
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets add it as a separate test case

@@ -100,6 +105,17 @@ class RichMetadataTest extends FlatSpec with TestCommon {
summaryMeta.containsSummaryMetadata shouldBe true
}

it should "create summary for a given metadata" in {
val richMetaData = RichMetadata(meta1)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to explicitely wrap metadata into RichMetadata. It should world implicitly.

Copy link
Collaborator

@tovbinm tovbinm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm. See comments.

@tovbinm tovbinm merged commit 06ca844 into salesforce:master Aug 20, 2018
@tovbinm
Copy link
Collaborator

tovbinm commented Aug 20, 2018

Ooops, I should have waited for the tests to complete.

@codecov
Copy link

codecov bot commented Aug 20, 2018

Codecov Report

Merging #72 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #72      +/-   ##
==========================================
+ Coverage   86.11%   86.18%   +0.06%     
==========================================
  Files         298      298              
  Lines        9303     9762     +459     
  Branches      304      531     +227     
==========================================
+ Hits         8011     8413     +402     
- Misses       1292     1349      +57
Impacted Files Coverage Δ
...a/com/salesforce/op/utils/spark/RichMetadata.scala 85.45% <100%> (+6.2%) ⬆️
.../scala/com/salesforce/op/cli/gen/ProblemKind.scala 100% <0%> (+100%) ⬆️
...src/main/scala/com/salesforce/op/cli/CliExec.scala 77.77% <0%> (+77.77%) ⬆️
...cala/com/salesforce/op/cli/gen/FileInProject.scala 100% <0%> (+100%) ⬆️
...in/scala/com/salesforce/op/cli/CliParameters.scala 80.95% <0%> (+80.95%) ⬆️
...a/com/salesforce/op/cli/gen/ProjectGenerator.scala 87.5% <0%> (+87.5%) ⬆️
...cala/com/salesforce/op/cli/gen/FileGenerator.scala 77.27% <0%> (+77.27%) ⬆️
...in/scala/com/salesforce/op/cli/gen/AvroField.scala 69.23% <0%> (+69.23%) ⬆️
...alesforce/op/cli/gen/templates/SimpleProject.scala 100% <0%> (+100%) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fc64f2a...7c3797f. Read the comment docs.

@gokulsfdc gokulsfdc deleted the RichMap_MetadataSeq branch August 20, 2018 16:13
ericwayman pushed a commit that referenced this pull request Feb 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants