Skip to content

Commit

Permalink
test: Skip delete old envelopes for most tests (#2656)
Browse files Browse the repository at this point in the history
Deleting old envelope items in the SentryFileManager can cause side effects for tests
as it could delete envelope items needed for other tests async. Now the SentryClient runs
the deletion in production and we only run the deletion for selected tests.
  • Loading branch information
philipphofmann authored Jan 26, 2023
1 parent 0dedab7 commit ecd9ecd
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 14 deletions.
2 changes: 2 additions & 0 deletions Sources/Sentry/SentryClient.m
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,8 @@ - (instancetype)initWithOptions:(SentryOptions *)options
self.timezone = timezone;
self.attachmentProcessors = [[NSMutableArray alloc] init];
self.deviceWrapper = deviceWrapper;

[fileManager deleteOldEnvelopeItems];
}
return self;
}
Expand Down
26 changes: 15 additions & 11 deletions Sources/Sentry/SentryFileManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
SentryFileManager ()

@property (nonatomic, strong) id<SentryCurrentDateProvider> currentDateProvider;
@property (nonatomic, strong) SentryDispatchQueueWrapper *dispatchQueue;
@property (nonatomic, copy) NSString *basePath;
@property (nonatomic, copy) NSString *sentryPath;
@property (nonatomic, copy) NSString *eventsPath;
Expand Down Expand Up @@ -59,9 +60,9 @@ - (nullable instancetype)initWithOptions:(SentryOptions *)options
dispatchQueueWrapper:(SentryDispatchQueueWrapper *)dispatchQueueWrapper
error:(NSError **)error
{
self = [super init];
if (self) {
if (self = [super init]) {
self.currentDateProvider = currentDateProvider;
self.dispatchQueue = dispatchQueueWrapper;
[self createPathsWithOptions:options];

// Remove old cached events for versions before 6.0.0
Expand All @@ -77,15 +78,6 @@ - (nullable instancetype)initWithOptions:(SentryOptions *)options

self.currentFileCounter = 0;
self.maxEnvelopes = options.maxCacheItems;

__weak SentryFileManager *weakSelf = self;
[dispatchQueueWrapper dispatchAsyncWithBlock:^{
if (weakSelf == nil) {
return;
}
SENTRY_LOG_DEBUG(@"Dispatched deletion of old envelopes from %@", weakSelf);
[weakSelf deleteOldEnvelopesFromAllSentryPaths];
}];
}
return self;
}
Expand All @@ -95,6 +87,18 @@ - (void)setDelegate:(id<SentryFileManagerDelegate>)delegate
_delegate = delegate;
}

- (void)deleteOldEnvelopeItems
{
__weak SentryFileManager *weakSelf = self;
[self.dispatchQueue dispatchAsyncWithBlock:^{
if (weakSelf == nil) {
return;
}
SENTRY_LOG_DEBUG(@"Dispatched deletion of old envelopes from %@", weakSelf);
[weakSelf deleteOldEnvelopesFromAllSentryPaths];
}];
}

- (void)deleteAllFolders
{
[self removeFileAtPath:self.sentryPath];
Expand Down
2 changes: 2 additions & 0 deletions Sources/Sentry/include/SentryFileManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ SENTRY_NO_INIT
- (void)deleteAllEnvelopes;
- (void)deleteAllFolders;

- (void)deleteOldEnvelopeItems;

/**
* Get all envelopes sorted ascending by the timeIntervalSince1970 the envelope was stored and if
* two envelopes are stored at the same time sorted by the order they were stored.
Expand Down
4 changes: 3 additions & 1 deletion Tests/SentryTests/Helper/SentryFileManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -137,17 +137,19 @@ class SentryFileManagerTests: XCTestCase {
try givenOldEnvelopes()

sut = fixture.getSut()
sut.deleteOldEnvelopeItems()

XCTAssertEqual(sut.getAllEnvelopes().count, 0)
}

func testDeleteOldEnvelopes_WithEmptyDSN() throws {
fixture.options.dsn = nil
sut = fixture.getSut()
sut.deleteOldEnvelopeItems()

try givenOldEnvelopes()

sut = fixture.getSut()
sut.deleteOldEnvelopeItems()

XCTAssertEqual(sut.getAllEnvelopes().count, 0)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class SentryWatchdogTerminationTrackerTests: NotificationCenterTestCase {
let appState = SentryAppState(releaseName: fixture.options.releaseName ?? "", osVersion: UIDevice.current.systemVersion, vendorId: TestData.someUUID, isDebugging: false, systemBootTimestamp: fixture.sysctl.systemBootTimestamp)

XCTAssertEqual(appState, actual)
XCTAssertEqual(2, fixture.dispatchQueue.dispatchAsyncCalled)
XCTAssertEqual(1, fixture.dispatchQueue.dispatchAsyncCalled)
}

func testGoToForeground_SetsIsActive() {
Expand All @@ -106,7 +106,7 @@ class SentryWatchdogTerminationTrackerTests: NotificationCenterTestCase {
goToBackground()

XCTAssertFalse(fixture.realFileManager.readAppState()?.isActive ?? true)
XCTAssertEqual(4, fixture.dispatchQueue.dispatchAsyncCalled)
XCTAssertEqual(3, fixture.dispatchQueue.dispatchAsyncCalled)
}

func testGoToForeground_WhenAppStateNil_NothingIsStored() {
Expand Down
8 changes: 8 additions & 0 deletions Tests/SentryTests/SentryClientTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,14 @@ class SentryClientTest: XCTestCase {
clearTestState()
}

func testInit_CallsDeleteOldEnvelopeItemsInvocations() throws {
let fileManager = try TestFileManager(options: Options())

_ = SentryClient(options: Options(), fileManager: fileManager)

XCTAssertEqual(1, fileManager.deleteOldEnvelopeItemsInvocations.count)
}

func testClientIsEnabled() {
XCTAssertTrue(fixture.getSut().isEnabled)
}
Expand Down
5 changes: 5 additions & 0 deletions Tests/SentryTests/TestClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,11 @@ class TestFileManager: SentryFileManager {
init(options: Options, andCurrentDateProvider currentDateProvider: CurrentDateProvider) throws {
try super.init(options: options, andCurrentDateProvider: currentDateProvider, dispatchQueueWrapper: TestSentryDispatchQueueWrapper())
}

var deleteOldEnvelopeItemsInvocations = Invocations<Void>()
override func deleteOldEnvelopeItems() {
deleteOldEnvelopeItemsInvocations.record(Void())
}

override func readTimestampLastInForeground() -> Date? {
readTimestampLastInForegroundInvocations += 1
Expand Down

0 comments on commit ecd9ecd

Please sign in to comment.