Integration Test framework and an initial test for Document Code #1291

steveyegge · 2024-04-06T21:59:40Z

This PR introduces the new JB integration test framework, based on the built-in JB testing facilities.

Overview

We only have one test so far, DocumentCodeTest, but:

It tests end-to-end communication between JetBrains and the Cody Agent over the jsonrpc protocol
It is fully asynchronous and can wait arbitrarily long for tests to complete
It can be configured to hit a production LLM; currently there is a stub mock LLM interaction
It has two separate validations:
- one protocol roundtrip (getFoldingRanges)
- one async client-side operation that awaits the appearance of the first codelens

This serves as a good reference example of using the test framework.

Note that one facility of the test framework is that we use the built-in JetBrains pubsub support to signal to subscribers (the tests) that certain async checkpoints have been reached, which the tests can await.

FixupSession and some other classes are instrumented with these notifications, and the tests subscribe to them.

Caveats

There is only one test and it's incomplete
I have not yet mocked out the LLM interaction for a full Document Code task/session
Even after that, there are many tests left to write.

Test plan

This is a test, so it's part of the test plan for previous PRs.

I have tried not to break anything, so I've verified that on this branch, when using the stevey/jetbrains-tests' branch of sourcegraph/cody`:

It's all still behind the CODY_JETBRAINS_FEATURES feature flag
Cody works
Integration test passes in both cli (gradlew intTest) and from within the IDE

We now have an environment-variable based feature flag, so the setting is no longer needed nor helpful.

figured out how to run the tests on the JUnit runner thread

LLM interaction is not simulated yet, but coming soon

It had disappeared while re-threading things for integration tests

required killing off the old agent process, which fixed some startup errors. More work to do

Issue was that Agent should not be in integrationi-test mode

There was a naming conflict before that has been resolved

pkukielka · 2024-04-08T07:19:26Z

src/main/kotlin/com/sourcegraph/cody/agent/CodyAgent.kt

@@ -97,10 +97,12 @@ private constructor(
        val conn = startAgentProcess()
        val client = CodyAgentClient()
        client.onSetConfigFeatures = project.service<CurrentConfigFeatures>()
+        logger.warn("Starting json-rpc Launcher")


Shouldn't all that logs be on the info level?

I added a few extra logging statements at key lifecycle stages of starting the Agent, because I found it tremendously helpful in narrowing down where it was failing when the Agent is pooping out. Which is often. I'll review them all, though, to see which ones we should really keep.

pkukielka · 2024-04-08T07:21:32Z

src/main/kotlin/com/sourcegraph/cody/agent/CodyAgent.kt

@@ -156,12 +159,27 @@ private constructor(
        processBuilder.environment()["CODY_LOG_EVENT_MODE"] = "connected-instance-only"
      }

+      if (ConfigUtil.isIntegrationTestModeEnabled()) {
+        // N.B. Do not set CODY_TESTING=true -- that is for Agent-side tests.
+        val testToken = System.getenv("CODY_INTEGRATION_TEST_TOKEN")


I think we should reuse tokens and polly.js recording infrastructure we already have in the agent.
This way we won't need to confugure/understand/setup two independent testing environments, and all token management could be taken care of on the agent side.

Some test conditions are hard to recreate with production backends. VSCode uses both kinds to good effect.

pkukielka · 2024-04-08T07:24:34Z

src/main/kotlin/com/sourcegraph/cody/agent/CodyAgentService.kt

@@ -47,24 +47,19 @@ class CodyAgentService(project: Project) : Disposable {
        FixupService.getInstance(project).getSessionForTask(task)?.update(task)
      }

-      agent.client.onEditTaskDidDelete = Consumer { task ->


Looks like you reverted my change to the callbacks/lifecycle managenet.
Was that intentional?
Initialising it in FixupService won't work in some cases, e.g. after agent restart.
If that was not intentional, please revert those changes.

@steveyegge pinging this comment ^

pkukielka · 2024-04-08T07:26:46Z

src/main/kotlin/com/sourcegraph/cody/edit/FixupService.kt

@@ -25,6 +27,71 @@ class FixupService(val project: Project) : Disposable {
  // The last text the user typed in without saving it, for continuity.
  private var lastPrompt: String = ""

+  init {


Please revert those changes or let's discuss offline.

dominiccooney

Putting this back in your queue. The diff looks odd to me—there's a bunch of inline edits changes, and @pkukielka mentioned one revert? in here.

If the edits changes are particularly conflicty, we could also land everything except the edits changes + edits test and drop a trivial hello world type test in there so we can get hacking on other integration tests in parallel with the edit test landing. But if it is close then let's not toil over splitting it up.

dominiccooney · 2024-04-20T22:03:56Z

build.gradle.kts

+    applySharedConfiguration(sharedIntegrationTestConfig)
+  }
+
+  // Make sure to set CODY_INTEGRATION_TEST_TOKEN when using this task.


onlyIf { System.env['CODY_INTEGRATION_TEST_TOKEN'] != null }

or let's conditionally print an error or something.

Ditto below.

dominiccooney · 2024-04-20T22:06:03Z

src/main/kotlin/com/sourcegraph/cody/agent/CodyAgentService.kt

@@ -47,24 +47,19 @@ class CodyAgentService(project: Project) : Disposable {
        FixupService.getInstance(project).getSessionForTask(task)?.update(task)
      }

-      agent.client.onEditTaskDidDelete = Consumer { task ->


@steveyegge pinging this comment ^

dominiccooney · 2024-04-20T22:21:12Z

This has conflicts with the actions refactoring in ff37c21. Very tempting to split this up so we can write other tests while closing the loop on edit tests.

we no longer cancel the FixupSession until the error lens is dismissed

restored a method call that had gone missing

moved the resource files into a testProjects directory

dominiccooney

It will be so great to get these going, can't wait to write some.

A few minor comments inline.

This needs rebasing to pick up the changes where 'edit' and 'workspace edit' produce a boolean.

dominiccooney · 2024-05-16T00:14:03Z

CONTRIBUTING.md

+the protocol changes.
+
+```
+CODY_INTEGRATION_TEST_TOKEN=sgp_asdfasdfasdfasdfasdfasdfasdf


There's a script in sourcegraph/cody, agent/scripts/export-cody-http-recording-tokens.sh . Can we standardize on using those tokens? Everyone has access to them and there are different variants for rate limited tokens, etc. if we need them down the road.

dominiccooney · 2024-05-16T00:17:23Z

build.gradle.kts

+          "CODY_JETBRAINS_FEATURES" to "cody.feature.inline-edits=true",
+          "CODY_RECORDING_MODE" to "replay",
+          "CODY_RECORDING_DIRECTORY" to "recordings",
+          "CODY_RECORD_IF_MISSING" to "false") // Polly needs this to record at all.


dominiccooney · 2024-05-16T00:17:42Z

build.gradle.kts

+          "CODY_RECORD_IF_MISSING" to "false") // Polly needs this to record at all.
+
+  // Common configuration for integration tests.
+  // TODO: This doesn't actually work.


Can you elaborate?

steveyegge and others added 21 commits March 22, 2024 00:23

broken integration test attempt

b451326

Make gradle integrationTests working

b5d19c9

initial steps towards putting Agent in integration test mode

ce239d6

Removed experimental user setting for inline edits

4d84e12

We now have an environment-variable based feature flag, so the setting is no longer needed nor helpful.

more work on integration tests

dacdd76

moved lens group visibility into the lens group

ccfee5b

got unblocked on the EDT issue

932b1d2

figured out how to run the tests on the JUnit runner thread

got first integration tests working

7dc4b37

LLM interaction is not simulated yet, but coming soon

fixed broken integration test

cfac44d

got integration tests working from cli/gradle

bac793b

more work on integration test framework

ded891e

fixed a deadlock in the tests

4edd3a1

removed an accidental comment

5bc49ba

fixed showing the Accept lens group

189dd04

It had disappeared while re-threading things for integration tests

added a TODO

fdff59f

merged with main

f2e011c

merged from main and got tests (almost) working again

5efd0c5

required killing off the old agent process, which fixed some startup errors. More work to do

lowered wait timeouts

1a946f7

some better error logging on timeouts

9eaa635

Got integration test working again

8cd8ffd

Issue was that Agent should not be in integrationi-test mode

improved a comment

16df516

steveyegge requested a review from pkukielka April 6, 2024 21:59

steveyegge mentioned this pull request Apr 7, 2024

Agent-side changes to support JetBrains integration tests sourcegraph/cody#3722

Open

steveyegge added 3 commits April 6, 2024 23:15

some hygiene and misc fixes

f8b8235

some refactoring of the pubsub code

a7243dd

renamed intTest to integrationTest

9665434

There was a naming conflict before that has been resolved

pkukielka reviewed Apr 8, 2024

View reviewed changes

fixed indentation

78de66c

dominiccooney requested changes Apr 20, 2024

View reviewed changes

steveyegge added 6 commits May 3, 2024 15:06

merged stevey/integration-tests with main

17df15e

a bit of work on error lenses

1a6793a

fixed some compile errors from the merge

366e3a3

fixed an EDT error

05d7f4a

fixes for error lenses

e1653c3

we no longer cancel the FixupSession until the error lens is dismissed

initial work on integration tests

842d42d

steveyegge force-pushed the stevey/integration-tests branch from d6f0473 to 842d42d Compare May 10, 2024 22:31

steveyegge added 12 commits May 10, 2024 15:36

merged main

ab26297

auto-accept on document changes

131cb36

restored a method call that had gone missing

minor refactoring cleanup in build file

b2252b7

more work on integration tests

b5b2812

some unfinished work on integrationTest

f9697e6

removed unused submodule declaration for jetbrains-shared

4dd4bf5

more work on integration tests

e0c1bd1

some refactoring of the integration test code

da9dbb9

moved the resource files into a testProjects directory

removed accidental redundant declaration

374dfec

added some assertions

926dc2b

moved .gitignore back to proper spot

64a501c

fixed problem with accidentally deleting our own source

764dbf9

dominiccooney approved these changes May 16, 2024

View reviewed changes

steveyegge added 7 commits May 16, 2024 16:03

Finished configuring integration tests from sources on disk

e763e34

added support for setting caret and selection range in test source files

14d07b9

merged main

62c8d29

fixed a compile error

7eeb1cd

fixed a bug where performing the inline edit was canceling the task

3f45fc1

comments

c5c2744

more work on integration test pubsub, not quite working yet

4d61876

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integration Test framework and an initial test for Document Code #1291

Integration Test framework and an initial test for Document Code #1291

steveyegge commented Apr 6, 2024 •

edited

pkukielka Apr 8, 2024

steveyegge Apr 16, 2024

pkukielka Apr 8, 2024 •

edited

dominiccooney Apr 8, 2024

pkukielka Apr 8, 2024

dominiccooney Apr 20, 2024

pkukielka Apr 8, 2024

dominiccooney left a comment

dominiccooney Apr 20, 2024

dominiccooney Apr 20, 2024

dominiccooney commented Apr 20, 2024

dominiccooney left a comment

dominiccooney May 16, 2024

dominiccooney May 16, 2024

dominiccooney May 16, 2024

Integration Test framework and an initial test for Document Code #1291

Are you sure you want to change the base?

Integration Test framework and an initial test for Document Code #1291

Conversation

steveyegge commented Apr 6, 2024 • edited

Overview

Caveats

Test plan

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pkukielka Apr 8, 2024 • edited

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

dominiccooney left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dominiccooney commented Apr 20, 2024

dominiccooney left a 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

steveyegge commented Apr 6, 2024 •

edited

pkukielka Apr 8, 2024 •

edited