Skip to content

Commit 90c306e

Browse files
author
vlussenburg
committed
Added testcase to prove erroneous behavior, and fixed it
1 parent 57f06f2 commit 90c306e

File tree

2 files changed

+13
-7
lines changed

2 files changed

+13
-7
lines changed

backtrace-library/src/androidTest/java/backtraceio/library/breadcrumbs/BacktraceBreadcrumbsTest.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
import java.io.File;
2020
import java.io.FileInputStream;
2121
import java.io.IOException;
22+
import java.nio.file.Files;
23+
import java.nio.file.Paths;
2224
import java.util.ArrayList;
2325
import java.util.HashMap;
2426
import java.util.LinkedHashMap;
@@ -308,8 +310,8 @@ public void testQueueFileShouldNotRollover() {
308310

309311
@Test
310312
public void testQueueFileRollover() {
313+
// Should reach the max of 64kB around breadcrumb 925
311314
final int numIterations = 1000;
312-
final int firstBreadcrumbIndexAfterRollover = 925;
313315
// Account for mandatory configuration breadcrumb
314316
backtraceBreadcrumbs.setCurrentBreadcrumbId(1);
315317

@@ -322,9 +324,12 @@ public void testQueueFileRollover() {
322324
backtraceBreadcrumbs.addBreadcrumb("I am a breadcrumb", attributes);
323325
}
324326

325-
List<String> breadcrumbLogFileData = readBreadcrumbLogFile();
327+
long breadcrumbsFileSize = Files.size(Paths.get(backtraceBreadcrumbs.getBreadcrumbLogPath()));
328+
assertTrue(String.format("Size of breadcrumbs file (%s) not close enough to a full breadcrumb file (%s)", breadcrumbsFileSize, 64 * 1024),
329+
breadcrumbsFileSize > 63 * 1024);
326330

327331
// We should have rolled over the configuration breadcrumb, consider all breadcrumbs here
332+
List<String> breadcrumbLogFileData = readBreadcrumbLogFile();
328333
for (int i = 0; i < breadcrumbLogFileData.size(); i++) {
329334
JSONObject parsedBreadcrumb = new JSONObject(breadcrumbLogFileData.get(i));
330335
assertEquals("I am a breadcrumb", parsedBreadcrumb.get("message"));
@@ -333,8 +338,9 @@ public void testQueueFileRollover() {
333338
assertEquals("info", parsedBreadcrumb.get("level"));
334339
// Timestamp should be convertible to a long
335340
assertTrue(parsedBreadcrumb.get("timestamp") instanceof Long);
336-
assertTrue(((int) parsedBreadcrumb.get("id")) > firstBreadcrumbIndexAfterRollover);
337-
assertTrue((int) parsedBreadcrumb.get("id") <= numIterations);
341+
final int id = (int) parsedBreadcrumb.get("id");
342+
assertTrue(String.format("Breadcrumb ID %s was higher than the expected numIterations %s",
343+
parsedBreadcrumb.get("id"), numIterations), id <= numIterations);
338344
}
339345

340346
} catch (Exception ex) {

backtrace-library/src/main/java/backtraceio/library/breadcrumbs/BacktraceQueueFileHelper.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,10 @@ public boolean add(byte[] bytes) {
6363
// In the 1.2.3 version we use, the usedBytes is int, not long, and the
6464
// implementation is synchronized thus safe for multithreaded access.
6565
// see https://github.com/square/tape/blob/tape-parent-1.2.3/tape/src/main/java/com/squareup/tape/QueueFile.java
66-
int usedBytes = (int) this.usedBytes.invoke(breadcrumbStore);
67-
while (!breadcrumbStore.isEmpty() && (usedBytes + breadcrumbLength) > maxQueueFileSizeBytes) {
66+
for (int usedBytes = (int) this.usedBytes.invoke(breadcrumbStore);
67+
!breadcrumbStore.isEmpty() && (usedBytes + breadcrumbLength) > maxQueueFileSizeBytes;
68+
usedBytes = (int) this.usedBytes.invoke(breadcrumbStore)) {
6869
breadcrumbStore.remove();
69-
usedBytes = (int) this.usedBytes.invoke(breadcrumbStore);
7070
}
7171

7272
breadcrumbStore.add(bytes);

0 commit comments

Comments
 (0)