You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Haley Reeve via Review Board <no...@reviews.apache.org> on 2019/03/04 15:48:51 UTC
Re: Review Request 70076: SENTRY-2505: Fix file bounds in
TestRollingFileWithoutDeleteAppender test case testFileNamePattern
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70076/#review213399
-----------------------------------------------------------
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/appender/TestRollingFileWithoutDeleteAppender.java
Line 89 (original), 89 (patched)
<https://reviews.apache.org/r/70076/#comment299341>
Do we understand why changing the bytes written works better?
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/appender/TestRollingFileWithoutDeleteAppender.java
Lines 93 (patched)
<https://reviews.apache.org/r/70076/#comment299340>
Doesn't this make the assertNotNull irrelevant? If files were null then accessing files.length would cause an error, so the assertNotNull must always be true when we reach it. Maybe we should keep the original order.
- Haley Reeve
On Feb. 28, 2019, 6:57 p.m., Arjun Mishra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70076/
> -----------------------------------------------------------
>
> (Updated Feb. 28, 2019, 6:57 p.m.)
>
>
> Review request for sentry, Haley Reeve, kalyan kumar kalvagadda, and Na Li.
>
>
> Bugs: SENTRY-2505
> https://issues.apache.org/jira/browse/SENTRY-2505
>
>
> Repository: sentry
>
>
> Description
> -------
>
> TestRollingFileWithoutDeleteAppender#testFileNamePattern still flaky because of size bounds
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/log/appender/TestRollingFileWithoutDeleteAppender.java 6ee6b08a0
>
>
> Diff: https://reviews.apache.org/r/70076/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Arjun Mishra
>
>