Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added Document Synchronization integration tests #1805

Open
wants to merge 48 commits into
base: main
Choose a base branch
from

Conversation

steveyegge
Copy link
Contributor

@steveyegge steveyegge commented Jun 25, 2024

This PR introduces a new set of integration tests for document synchronization testing.

This PR is dependent on a Cody-side PR, and the tests will not pass until that PR is merged.

Test plan

This is not ready to merge yet; I'll update this section when it's ready to go.

this.javaClass.getMethod(methodName)
?: throw IllegalStateException(
"No method with name $methodName found in ${this.javaClass.name}")
if (method.isAnnotationPresent(TestFile::class.java)) {
Copy link
Contributor

@pkukielka pkukielka Jun 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could use the default test file if no annotation is added?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call; I'll clean that up.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you missed this one @steveyegge

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, there really isn't a good choice for a default right now, since the only test file is specific to DocumentCodeTest.

But I went ahead and used that as the default for now, until we get more tests with disk files.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to roll this change back because it broke all the new tests, which don't have testResources files on disk.

Copy link
Contributor

@pkukielka pkukielka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added one comment, otherwise LGTM

@steveyegge steveyegge force-pushed the stevey/doc-synchronization-tests branch from e050427 to bda3bbe Compare June 26, 2024 00:48
@steveyegge steveyegge changed the title Attempting to let test methods have their own test files added Document Synchronization integration tests Jun 27, 2024
@steveyegge steveyegge enabled auto-merge (squash) June 27, 2024 02:15
@steveyegge steveyegge self-assigned this Jun 27, 2024
@steveyegge steveyegge force-pushed the stevey/doc-synchronization-tests branch from c56438e to f752745 Compare June 27, 2024 02:29
Copy link
Contributor

@dominiccooney dominiccooney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Note, one bit of feedback from @pkukielka you said you'll address.

@@ -18,16 +19,25 @@ import org.junit.runners.Suite
* automatically after the platform version bump.
*
* Multiple recording files can be used, but each should have its own suite with tearDown() method
* nad define unique CODY_RECORDING_NAME.
* and define unique CODY_RECORDING_NAME.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...while we're here...

Suggested change
* and define unique CODY_RECORDING_NAME.
* and define a unique CODY_RECORDING_NAME.

"""
class Foo {
console.log("hello there!🎉🎂
🥳🎈")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resulting indentation is interesting...

@@ -42,7 +42,8 @@ open class CodyIntegrationTextFixture : BasePlatformTestCase() {

override fun tearDown() {
try {
FixupService.getInstance(myFixture.project).getActiveSession()?.apply {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't that whole file be removed and that line modified in the CodyIntegrationTextFixture.kt?
Then git should correctly show it as rename, and not like now as whole new file added.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch; fixed that.

@steveyegge steveyegge force-pushed the stevey/doc-synchronization-tests branch 3 times, most recently from 014fd7f to 6340bfd Compare July 1, 2024 15:41
@steveyegge steveyegge force-pushed the stevey/doc-synchronization-tests branch from e068545 to dddffa7 Compare July 2, 2024 15:30
steveyegge and others added 19 commits July 2, 2024 08:52
## Test plan

Add account that exists 
1. Using settings, add the account that already exists in the list
… having a pro llm selected before (#1866)

## Test plan
1. Having a pro account
2. switch to a pro llm in the chat
3. remove the token from the web 
4. re-authenticate via github
Closes
https://linear.app/sourcegraph/issue/CODY-988/p1-authorization-and-log-in-log-out

## Test plan

Repeat for dotcom and enterprise user account:

**Scenario 1** 

1. Remove all existing accounts
2. Make sure you are redirected to the login panel
3. Login using one of the available providers
4. Make sure that both `cody.auth.login/clicked` and
`cody.auth.signin.token/clicked` telemetry events were fired

**Scenario 2** 

1. Click on Cody status bar and choose `Manage Accounts`
2. Click to add new account
3. Make sure that  `cody.auth.login/clicked` was fired
4. Fill the account details (server and token) and confirm 
5.  Make sure that  `cody.auth.signin.token/clicked` was fired
No functional changes. Features should work as before.

## Test plan
1. Sign in panel features working
## Test plan
Delete the token from the keychain externally 
You can run a separate instance of IDE and remove the account there.

1. Log in to the account
2. Open another IDE, log in to the same account, apply, then remove the
account, apply
3. Switch back to the first ide
4. Close and reopen the project
5. Login panel visible
## Test plan
1. `Sign In with <>` and `Get started` buttons work
…where (#1872)

Closes
https://linear.app/sourcegraph/issue/CODY-2130/improve-how-actions-appear-in-shiftshift-menu

## Changes

I moved action properties like `text` and `description` to `plugin.xml`.
It is just easier to edit and inspect this way.
To be honest, Geminit 1.5 did most of the work:

> Given the following plugin.xml file and properties file, please modify
plugin.xml so each action have additional propertes `text` and
`description` with values from properties file.
> Additionally for every action please add such tag: `<override-text
place="GoToAction" text="Cody: $TEXT"/>` where `$TEXT` is taken from
properties file as well.
> For some actions like: `<action
id="Cody.Accounts.LogInToSourcegraphAction"
class="com.sourcegraph.cody.config.LogInToSourcegraphAction"/>` you may
have to change them to form such as:
> ```
> <action id="Cody.Accounts.LogInToSourcegraphAction"
class="com.sourcegraph.cody.config.LogInToSourcegraphAction" text="Log
In to Sourcegraph">
> <override-text place="GoToAction" text="Cody: Log In to Sourcegraph"/>
> </action>
>```
>
> Please output whole content of the modified plugin.xml file.


## Test plan

1. Hit `Shift shift`
2. Type 'Cody:'


![image](https://github.com/sourcegraph/jetbrains/assets/1519649/5ffd1d07-0088-4cb6-b5c8-3894b281bbe1)
1. Bump cody commit to include changes from
sourcegraph/cody#4766
2. Add startup telemetry event.
I realised that only v2 telemetry events sent directly from IJ contains
info about IDE version and we want to have at least one such event for
every user IDE. Install action telemetry event would not be sufficient
because it would not tell us anything about existing installations.

Same as in sourcegraph/cody#4766
Add the missing value for the ollama docs

## Test plan

<!-- All pull requests REQUIRE a test plan:
https://sourcegraph.com/docs/dev/background-information/testing_principles

Why does it matter?

These test plans are there to demonstrate that are following industry
standards which are important or critical for our customers.
They might be read by customers or an auditor. There are meant be simple
and easy to read. Simply explain what you did to ensure
your changes are correct!

Here are a non exhaustive list of test plan examples to help you:

- Making changes on a given feature or component:
- "Covered by existing tests" or "CI" for the shortest possible plan if
there is zero ambiguity
  - "Added new tests"
- "Manually tested" (if non trivial, share some output, logs, or
screenshot)
- Updating docs:
  - "previewed locally"
  - share a screenshot if you want to be thorough
- Updating deps, that would typically fail immediately in CI if
incorrect
  - "CI"
  - "locally tested"
-->

![Screenshot 2024-07-03 at 12 19
49 PM](https://github.com/sourcegraph/jetbrains/assets/68532117/bb29c397-a40e-453f-b8bf-010e169de863)
Our current setup produces a lot of stable releases that often are not
unhidden eventually. That makes the QA and awaiting JB approval process
parallel. Based on the recent experience only 1/8 releases goes public.
That is a waste of JB approval team's time and CI time.

Let's change the setup. The nightly version handling does not change but
the default release script does not longer publish the stable version.
Instead the separate workflow can be triggered on the specific tag that
publishes the stable version.

## Test plan
1. push a tag for a nightly release
2. trigger the stable release manually

to be tested once merged
Closes
https://linear.app/sourcegraph/issue/CODY-2095/organize-keymap-settings

## Changes

1. Proper grouping was added for all actions in plugin.xml
2. The same grouping is used for nice visual organisation of keymap
management

## Test plan

Visual verification in `Settings > Keymap` (see screenshots):


![image](https://github.com/sourcegraph/jetbrains/assets/1519649/b5292cfb-5a2f-47ae-9706-512fb2675b8a)

![image](https://github.com/sourcegraph/jetbrains/assets/1519649/5553077a-c21e-4003-b1c8-0c2f279f6467)
## Test plan

Full QA according to a test plan.
As for launch script args change that is covered by automatic tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants