From 7806cfe81f8c9a8081b82211d3a7722ba93e3900 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Thu, 20 Jun 2024 14:13:05 -0700 Subject: [PATCH] Fix a nondeterministic test failure of `testDontStackTargetPreparationForEditorFunctionality` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We fulfilled the `allDocumentsOpened` expectation directly after dispatching the notification to open LibD.swift. This left a very short window in which LibB’s preparation could finish and LibC’s expectation could start before the open notification for LibD.swift was handled, causing an unexpected preparation of LibC. This fixes the issue first reported as rdar://129698768 and for which I added logging in https://github.com/apple/sourcekit-lsp/pull/1478. --- .../BackgroundIndexingTests.swift | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift index bd657aab5..9cfffb851 100644 --- a/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift +++ b/Tests/SourceKitLSPTests/BackgroundIndexingTests.swift @@ -693,7 +693,16 @@ final class BackgroundIndexingTests: XCTestCase { DidChangeWatchedFilesNotification(changes: [FileEvent(uri: try project.uri(for: "LibA.swift"), type: .changed)]) ) - // Quickly flip through all files + // Quickly flip through all files. The way the test is designed to work is as follows: + // - LibB.swift gets opened and prepared. Preparation is simulated to take a long time until both LibC.swift and + // LibD.swift have been opened. + // - LibC.swift gets opened. This queues preparation of LibC but doesn't cancel preparation of LibB because we + // don't cancel in-progress preparation tasks to guarantee forward progress (see comment at the end of + // `SemanticIndexManager.prepare`). + // - Now LibD.swift gets opened. This cancels preparation of LibC which actually cancels LibC's preparation for + // real because LibC's preparation hasn't started yet (it's only queued). + // Thus, the only targets that are being prepared are LibB and LibD, which is checked by the + // `ExpectedIndexTaskTracker`. _ = try project.openDocument("LibB.swift") try libBStartedPreparation.waitOrThrow() @@ -705,6 +714,9 @@ final class BackgroundIndexingTests: XCTestCase { _ = try await project.testClient.send(BarrierRequest()) _ = try project.openDocument("LibD.swift") + // Send a barrier request to ensure we have finished opening LibD before allowing the preparation of LibB to finish. + _ = try await project.testClient.send(BarrierRequest()) + allDocumentsOpened.signal() try libDPreparedForEditing.waitOrThrow() }