Add integration tests for the chat (context files) #1930

mkondratek · 2024-07-22T11:09:12Z

This is a follow up PR for #1928.

TODO

cody commit update

Test plan

Green CI

src/integrationTest/kotlin/com/sourcegraph/cody/chat/ChatTest.kt

pkukielka · 2024-07-23T12:08:18Z

src/main/kotlin/com/sourcegraph/cody/chat/ui/MessagesPanel.kt

@@ -54,7 +54,12 @@ class MessagesPanel(private val project: Project, private val chatSession: ChatS

  @RequiresEdt
  fun removeBlinkingCursor() {
-    components.find { it is BlinkingCursorComponent }?.let { remove(it) }
+    components
+        .find { it is BlinkingCursorComponent }


You could use as? - you won't need to cast it twice.

pkukielka · 2024-07-23T12:10:12Z

src/integrationTest/kotlin/com/sourcegraph/cody/chat/ChatTest.kt

+        val repos = agent.server.getRepoIds(param).get()
+        result.complete(repos?.repos ?: emptyList())
+      } catch (e: Exception) {
+        result.complete(emptyList())


Maybe we can replace it with assert with exception message logged out?
This way if it will fail we will also know why.

pkukielka · 2024-07-23T12:16:46Z

src/integrationTest/kotlin/com/sourcegraph/cody/chat/ChatTest.kt

+
+    val session = runInEdtAndGet {
+      val mySession = AgentChatSession.createNew(project)
+      mySession.sendWebviewMessage(


I'm wondering if we couldn't do it using mySession.getPanel().contextView.updateFromSavedState (or updateFromAgent)?
It would test even more components this way.

mkondratek · 2024-07-23T13:44:54Z

src/main/kotlin/com/sourcegraph/cody/chat/ui/BlinkingCursorComponent.kt

 import com.intellij.util.ui.UIUtil
 import java.awt.Dimension
 import java.awt.Font
 import java.awt.Graphics
 import javax.swing.JPanel
 import javax.swing.Timer

-class BlinkingCursorComponent private constructor() : JPanel() {
+class BlinkingCursorComponent : JPanel(), Disposable {


Test execution threw this error:

Not disposed javax.swing.Timer: Timer (listeners: [com.sourcegraph.cody.chat.ui.BlinkingCursorComponent$$Lambda$1647/0x0000000801020c40@5a35a08]) (delayed for 482ms); queue:TimerQueue (javax.swing.Timer@5d0d7a2b) junit.framework.AssertionFailedError: Not disposed javax.swing.Timer: Timer (listeners: [com.sourcegraph.cody.chat.ui.BlinkingCursorComponent$$Lambda$1647/0x0000000801020c40@5a35a08]) (delayed for 482ms); queue:TimerQueue (javax.swing.Timer@5d0d7a2b) at com.intellij.testFramework.TestApplicationManagerKt.checkJavaSwingTimersAreDisposed(TestApplicationManager.kt:355) at com.intellij.testFramework.TestApplicationManagerKt.tearDownProjectAndApp(TestApplicationManager.kt:207) at com.intellij.testFramework.fixtures.impl.LightIdeaTestFixtureImpl.lambda$tearDown$3(LightIdeaTestFixtureImpl.java:79) at com.intellij.testFramework.RunAllKt.doRun(runAll.kt:50) at com.intellij.testFramework.RunAllKt.access$doRun(runAll.kt:1) at com.intellij.testFramework.RunAll.run(runAll.kt:17) at com.intellij.testFramework.fixtures.impl.LightIdeaTestFixtureImpl.tearDown(LightIdeaTestFixtureImpl.java:105) at com.intellij.testFramework.fixtures.impl.CodeInsightTestFixtureImpl.lambda$tearDown$33(CodeInsightTestFixtureImpl.java:1326) at com.intellij.testFramework.EdtTestUtil.lambda$runInEdtAndWait$1(EdtTestUtil.java:40) at com.intellij.openapi.application.TransactionGuardImpl.runWithWritingAllowed(TransactionGuardImpl.java:215) at com.intellij.openapi.application.TransactionGuardImpl.access$100(TransactionGuardImpl.java:22) at com.intellij.openapi.application.TransactionGuardImpl$1.run(TransactionGuardImpl.java:197) at com.intellij.openapi.application.impl.ApplicationImpl.runIntendedWriteActionOnCurrentThread(ApplicationImpl.java:873) at com.intellij.openapi.application.impl.ApplicationImpl$3.run(ApplicationImpl.java:511) at com.intellij.openapi.application.impl.LaterInvocator$1.run(LaterInvocator.java:96) at com.intellij.openapi.application.impl.FlushQueue.doRun(FlushQueue.java:69) at com.intellij.openapi.application.impl.FlushQueue.runNextEvent(FlushQueue.java:112) at com.intellij.openapi.application.impl.FlushQueue.flushNow(FlushQueue.java:42) at java.desktop/java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:313) at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:776) at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:727) at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:721) at java.base/java.security.AccessController.doPrivileged(Native Method) at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85) at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:746) at com.intellij.ide.IdeEventQueue.defaultDispatchEvent(IdeEventQueue.java:898) at com.intellij.ide.IdeEventQueue._dispatchEvent(IdeEventQueue.java:746) at com.intellij.ide.IdeEventQueue.lambda$dispatchEvent$6(IdeEventQueue.java:439) at com.intellij.openapi.progress.impl.CoreProgressManager.computePrioritized(CoreProgressManager.java:803) at com.intellij.ide.IdeEventQueue.lambda$dispatchEvent$7(IdeEventQueue.java:438) at com.intellij.openapi.application.TransactionGuardImpl.performActivity(TransactionGuardImpl.java:119) at com.intellij.ide.IdeEventQueue.performActivity(IdeEventQueue.java:604) at com.intellij.ide.IdeEventQueue.lambda$dispatchEvent$8(IdeEventQueue.java:436) at com.intellij.openapi.application.impl.ApplicationImpl.runIntendedWriteActionOnCurrentThread(ApplicationImpl.java:873) at com.intellij.ide.IdeEventQueue.dispatchEvent(IdeEventQueue.java:484) at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:207) at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:128) at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:117) at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:113) at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:105) at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:92)

odisseus · 2024-07-24T00:08:41Z

src/main/kotlin/com/sourcegraph/cody/chat/ui/MessagesPanel.kt

+    components
+        .firstNotNullOfOrNull { it as? BlinkingCursorComponent }
+        ?.let {
+          it.dispose()


The Jetbrains documentation says that Disposable.dispose() should never be called directly. However, it turns out that we do this a lot in our source code, and I don't expect this PR to rectify all those occurrences.

good point - I fixed it in this place. I will review the other places later.

odisseus · 2024-07-24T08:44:59Z

src/integrationTest/kotlin/com/sourcegraph/cody/chat/ChatTest.kt

+    await.atMost(15, TimeUnit.SECONDS) until
+        {
+          val messages = session.messages
+          messages.size == 2 &&
+              !messages[0].contextFiles.isNullOrEmpty() &&
+              !messages[1].text.isNullOrBlank()
+        }


I don't like this style of assertions.

If the condition doesn't hold, it will just throw a timeout exception, regardless of the failure mode. I would want to distinguish between an unexpected response (e.g. 3 messages instead of 2), and a complete lack of response.

Moreover, if I understand the documentation correctly, this code will wait for 15 seconds even if the response arrives immediately.

Moreover, if I understand the documentation correctly, this code will wait for 15 seconds even if the response arrives immediately.

This is not true. atMost waits at most the given time. Having the recordings ready the response arrives in <1s.

Okay, but I still think it would be nice to distinguish between different failure modes.

odisseus · 2024-07-24T09:05:47Z

There are some minor style issues, otherwise looks good.

## Test plan 1. #1930 passing (so we are sure the fix to the enhanced context works)

…ame with it

This reverts commit b4672ed.

Reverts #1930 Changes not ready yet. ## Test plan - Green CI

mkondratek self-assigned this Jul 22, 2024

mkondratek marked this pull request as draft July 22, 2024 11:09

mkondratek changed the title ~~Fix context file items presentation~~ Add integration tests for Chat (context files) Jul 22, 2024

mkondratek changed the title ~~Add integration tests for Chat (context files)~~ Add integration tests for the chat (context files) Jul 22, 2024

mkondratek force-pushed the mkondratek/it/context-files branch from f04aa6e to bcd944b Compare July 22, 2024 11:10

pkukielka reviewed Jul 22, 2024

View reviewed changes

src/integrationTest/kotlin/com/sourcegraph/cody/chat/ChatTest.kt Outdated Show resolved Hide resolved

mkondratek force-pushed the mkondratek/it/context-files branch from bcd944b to 9dbb68b Compare July 23, 2024 11:24

mkondratek marked this pull request as ready for review July 23, 2024 12:07

pkukielka reviewed Jul 23, 2024

View reviewed changes

mkondratek commented Jul 23, 2024

View reviewed changes

mkondratek requested a review from pkukielka July 23, 2024 14:06

odisseus self-requested a review July 24, 2024 00:01

odisseus reviewed Jul 24, 2024

View reviewed changes

odisseus approved these changes Jul 24, 2024

View reviewed changes

mkondratek force-pushed the mkondratek/it/context-files branch 2 times, most recently from 2510fe9 to 8100231 Compare July 31, 2024 07:28

mkondratek enabled auto-merge (squash) July 31, 2024 07:28

mkondratek force-pushed the mkondratek/it/context-files branch from 8100231 to 98ad518 Compare July 31, 2024 10:03

mkondratek mentioned this pull request Aug 6, 2024

A few minor fixes #1983

Merged

mkondratek added a commit that referenced this pull request Aug 6, 2024

A few minor fixes (#1983)

5c3d93b

## Test plan 1. #1930 passing (so we are sure the fix to the enhanced context works)

mkondratek force-pushed the mkondratek/it/context-files branch from f1f6878 to c8bd173 Compare August 7, 2024 11:38

mkondratek mentioned this pull request Aug 8, 2024

Move the printing of recordings directly to testing/requestErrors sourcegraph/cody#5142

Closed

mkondratek force-pushed the mkondratek/it/context-files branch 2 times, most recently from 1840ade to e9f6b27 Compare August 8, 2024 11:58

mkondratek added 2 commits August 9, 2024 14:10

Add integration test

cdb4ac4

Fixes

535972c

mkondratek added 6 commits August 9, 2024 14:10

Fixes

8af21db

Fixes

61dd8e4

Fixes

524ed82

Fixes

b73eebd

Fixes & recordings update

d4786d5

Fix testingRequestErrors call

be2c598

mkondratek force-pushed the mkondratek/it/context-files branch from e9f6b27 to 35615c4 Compare August 9, 2024 12:50

Add additionalEnvs param for the agent process & pass the recording n…

7ff9035

…ame with it

mkondratek force-pushed the mkondratek/it/context-files branch from 35615c4 to 7ff9035 Compare August 9, 2024 12:52

mkondratek merged commit b4672ed into main Aug 9, 2024
6 of 7 checks passed

mkondratek deleted the mkondratek/it/context-files branch August 9, 2024 13:02

mkondratek added a commit that referenced this pull request Aug 10, 2024

Revert "Add integration tests for the chat (context files) (#1930)"

6223b5c

This reverts commit b4672ed.

mkondratek mentioned this pull request Aug 10, 2024

Revert "Add integration tests for the chat (context files)" #2030

Merged

mkondratek added a commit that referenced this pull request Aug 10, 2024

Revert "Add integration tests for the chat (context files)" (#2030)

53f24b0

Reverts #1930 Changes not ready yet. ## Test plan - Green CI

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add integration tests for the chat (context files) #1930

Add integration tests for the chat (context files) #1930

mkondratek commented Jul 22, 2024 •

edited

Loading

pkukielka Jul 23, 2024

mkondratek Jul 23, 2024

pkukielka Jul 23, 2024

mkondratek Jul 23, 2024

pkukielka Jul 23, 2024 •

edited

Loading

mkondratek Jul 23, 2024

mkondratek Jul 23, 2024

odisseus Jul 24, 2024

mkondratek Jul 30, 2024

odisseus Jul 24, 2024

mkondratek Jul 30, 2024

odisseus Jul 30, 2024

mkondratek Jul 30, 2024

odisseus commented Jul 24, 2024

Add integration tests for the chat (context files) #1930

Add integration tests for the chat (context files) #1930

Conversation

mkondratek commented Jul 22, 2024 • edited Loading

TODO

Test plan

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pkukielka Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

odisseus commented Jul 24, 2024

mkondratek commented Jul 22, 2024 •

edited

Loading

pkukielka Jul 23, 2024 •

edited

Loading