[GitHub] spark pull request #20636: [SPARK-23415][SQL][TEST] Make behavior of BufferH...

[srowen <gi...@git.apache.org>]

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

    https://github.com/apache/spark/pull/20636#discussion_r206638094
  
    --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/BufferHolderSparkSubmitSuite.scala ---
    @@ -55,22 +55,30 @@ object BufferHolderSparkSubmitSuite {
     
         val ARRAY_MAX = ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH
     
    -    val holder = new BufferHolder(new UnsafeRow(1000))
    +    val unsafeRow = new UnsafeRow(1000)
    +    val holder = new BufferHolder(unsafeRow)
     
         holder.reset()
    -    holder.grow(roundToWord(ARRAY_MAX / 2))
     
    -    holder.reset()
    -    holder.grow(roundToWord(ARRAY_MAX / 2 + 8))
    +    // while to reuse a buffer may happen, this test checks whether the buffer can be grown
    +    holder.grow(ARRAY_MAX / 2)
    +    assert(unsafeRow.getSizeInBytes % 8 == 0)
     
    -    holder.reset()
    -    holder.grow(roundToWord(Integer.MAX_VALUE / 2))
    +    holder.grow(ARRAY_MAX / 2 + 7)
    +    assert(unsafeRow.getSizeInBytes % 8 == 0)
     
    -    holder.reset()
    -    holder.grow(roundToWord(Integer.MAX_VALUE))
    -  }
    +    holder.grow(Integer.MAX_VALUE / 2)
    +    assert(unsafeRow.getSizeInBytes % 8 == 0)
    +
    +    holder.grow(ARRAY_MAX - holder.totalSize())
    +    assert(unsafeRow.getSizeInBytes % 8 == 0)
     
    -  private def roundToWord(len: Int): Int = {
    -    ByteArrayMethods.roundNumberOfBytesToNearestWord(len)
    +    try {
    +      holder.grow(ARRAY_MAX + 1 - holder.totalSize())
    +      assert(false)
    +    } catch {
    +        case _: UnsupportedOperationException => assert(true)
    --- End diff --
    
    Fix the indents here. `assert(true)` is a no-op, so just omit it. `assert(false)` is less useful than `fail(...message...)`, above. Let an unexpected `Throwable` just fly out of the method to fail it rather than swallow it. But do you really just want to use `intercept` here?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org