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.

Sign up for GitHub

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

Jump to bottom

fix(lsp): Add test harness, update named arg code action title, fix code action trigger points, do not print LSP warnings #2134

Merged
merged 2 commits into from
Aug 23, 2024
Merged

fix(lsp): Add test harness, update named arg code action title, fix code action trigger points, do not print LSP warnings #2134

merged 2 commits into from
Aug 23, 2024

Conversation

alex-snezhko
Copy link
Member

  • Add LSP test harness, test most major LSP functionality
  • Make some improvements to LSP behavior according to expected behavior in tests

ospencer
ospencer reviewed Aug 4, 2024
View reviewed changes
Copy link
Member

@ospencer ospencer left a comment

Choose a reason for hiding this comment

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

Pretty cool! Can you specify the behavior improvements in the PR title?

@alex-snezhko alex-snezhko changed the title fix(lsp): LSP tests, behavior improvements fix(lsp): LSP tests, improvements: update named arg code action title, fix named arg and type annotation trigger points, do not print LSP warnings Aug 4, 2024
@alex-snezhko
Copy link
Member Author

@ospencer updated

@ospencer
Copy link
Member

ospencer commented Aug 4, 2024

Looks like you've got some line ending issues on Windows.

@ospencer ospencer changed the title fix(lsp): LSP tests, improvements: update named arg code action title, fix named arg and type annotation trigger points, do not print LSP warnings fix(lsp): Add test harness, update named arg code action title, fix code action trigger points, do not print LSP warnings Aug 4, 2024
marcusroberts
marcusroberts approved these changes Aug 8, 2024
View reviewed changes
Copy link
Member

@marcusroberts marcusroberts left a comment

Choose a reason for hiding this comment

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

It's great to get a test harness in place like this.

Other fixes look good too!

spotandjake
spotandjake reviewed Aug 15, 2024
View reviewed changes
Copy link
Member

@spotandjake spotandjake left a comment

Choose a reason for hiding this comment

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

This looks great, I am wondering if we should allow for the option of a snapshotRunner for some of these tests.

This also closes #1798

@spotandjake spotandjake added tests lsp Issues related to the language server. labels Aug 19, 2024
@alex-snezhko alex-snezhko force-pushed the alex/lsp-tests branch 4 times, most recently from b6cdfe3 to fc402ee Compare August 22, 2024 05:07
@alex-snezhko
Copy link
Member Author

@ospencer I fixed the Windows issues

@ospencer ospencer added this pull request to the merge queue Aug 23, 2024
Merged via the queue into main with commit 36c0bb8 Aug 23, 2024
12 checks passed
@ospencer ospencer deleted the alex/lsp-tests branch August 23, 2024 05:35
@github-actions github-actions bot mentioned this pull request Aug 17, 2024
Open
@spotandjake spotandjake mentioned this pull request Oct 28, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spotandjake spotandjake spotandjake left review comments

@marcusroberts marcusroberts marcusroberts approved these changes

@ospencer ospencer ospencer approved these changes

@phated phated Awaiting requested review from phated phated is a code owner

@peblair peblair Awaiting requested review from peblair peblair is a code owner

Assignees
No one assigned
Labels
lsp Issues related to the language server. tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@alex-snezhko @ospencer @marcusroberts @spotandjake