[GitHub] [incubator-druid] jnaous commented on issue #8909: Optimize CachingLocalSegmentAllocator#getSequenceName

[GitBox <gi...@apache.org>]

jnaous commented on issue #8909: Optimize CachingLocalSegmentAllocator#getSequenceName
URL: https://github.com/apache/incubator-druid/pull/8909#issuecomment-566852861
 
 
   +1 for driving to a conclusion about test name format. FWIW, I like
   Suneet's explicit format.
   
   On Tue, Dec 17, 2019, 5:17 PM Jihoon Son <no...@github.com> wrote:
   
   > *@jihoonson* commented on this pull request.
   > ------------------------------
   >
   > In
   > indexing-service/src/test/java/org/apache/druid/indexing/common/task/RangePartitionCachingLocalSegmentAllocatorTest.java
   > <https://github.com/apache/incubator-druid/pull/8909#discussion_r359110561>
   > :
   >
   > > @@ -141,6 +142,16 @@ public void allocatesCorrectShardSpecsForLastPartition()
   >      testAllocate(row, interval, partitionNum, null);
   >    }
   >
   > +  @Test
   > +  public void test_getSequenceName_forIntervalAndRow_shouldUseISOFormatAndPartitionNumForRow()
   >
   > Hmm, I would just use getSequenceName() for these tests with a comment
   > for the expected value because they look simple enough to understand.
   >
   > can you recommend a name? I like this format because it's explicit about
   > what the test is doing
   >
   > test_<functionUnderTest>_<condition>_<expectedResult>
   >
   > I agree on that we could have a better format or a better practice for
   > test names (we've been talking about a better practice for testing here
   > <https://www.mail-archive.com/dev@druid.apache.org/msg02426.html>).
   > Unfortunately, we don't have such a format or practice and I think it could
   > be better to make them consistent for now because we don't know what kind
   > of policy we would adopt yet. I think it would be worth to start a
   > discussion on it.
   >
   > —
   > You are receiving this because you are subscribed to this thread.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/incubator-druid/pull/8909?email_source=notifications&email_token=AAPSYCW5UHDOJIWUGKAC64TQZF22TA5CNFSM4JPINE22YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCPRY77A#discussion_r359110561>,
   > or unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AAPSYCVA3LVQOFZ7KI4HAPLQZF22TANCNFSM4JPINE2Q>
   > .
   >
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org