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

LSP ProjectView test + fixes #3502

Merged
merged 4 commits into from
Jan 30, 2022
Merged

Conversation

sdedic
Copy link
Member

@sdedic sdedic commented Jan 25, 2022

This PR focus on integration-testing the projects view. It checks basic scenarios: created files appear, deleted disappear and the package nodes restructure according to the occupied folders.

I moved certain registrations / layer folders from nbcode integration module to LSP module - these are harmless for IDE execution, but the LSP server (which can be potentially started inside NB IDE) does not function without those folders well. Also foundProjects view is defined in the LSP server but its critical cookie registrations were in the integration module - a different distribution.

During testing, I've encountered a NPE thrown when the client asks for details of a non-existing node: this is fixed by the last commit.

@sdedic sdedic added kind:bug Bug report or fix LSP [ci] enable Language Server Protocol tests labels Jan 25, 2022
@sdedic sdedic added this to the NB14 milestone Jan 25, 2022
@sdedic sdedic self-assigned this Jan 25, 2022
Copy link

@JaroslavTulach JaroslavTulach left a comment

Choose a reason for hiding this comment

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

It is sad that two biggest files in the change are gradlew wrappers! I'd prefer to generate them using "gradle init" which is available when one uses "New From Template" on the gradle project. I know you argued that JAR files should rather be in the repository and this is similar issue, so I leave the decision up to you.

Copy link
Contributor

@dbalek dbalek left a comment

Choose a reason for hiding this comment

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

Looks fine.

@sdedic sdedic merged commit 6577f78 into apache:master Jan 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug Bug report or fix LSP [ci] enable Language Server Protocol tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants