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

Fix #1316 - Vue.JS tests failures #1324

Merged
merged 1 commit into from
Sep 11, 2023

Conversation

zulus
Copy link
Contributor

@zulus zulus commented Sep 6, 2023

Real fix is in TestVue.java (-1 is no longer required)

In the meantime I also updated server and remove unnecessary System.out.println

@vrubezhny
Copy link
Contributor

/request-license-review

@vrubezhny
Copy link
Contributor

@zulus Still the same errors when building&testing on Jenkins as well as when I do it locally.

@zulus
Copy link
Contributor Author

zulus commented Sep 6, 2023

strange, I made a lot of successful tests before commit. I'll try again

@zulus zulus force-pushed the bug-1316 branch 4 times, most recently from 96f8bc4 to 8729819 Compare September 11, 2023 13:36
@zulus
Copy link
Contributor Author

zulus commented Sep 11, 2023

Problem appeared after modification on LSP4E side: eclipse-lsp4e/lsp4e@7c770c3

In general problem is with LSPEclipseUtils.toCompletionParams

  1. it always sends triggerCharacter
  2. force sends CompletionTriggerKind.TriggerCharacter i char is on potential trigger list (is not possible to change this)

Due this vue server on top of recent version lost a lot of potential completions, other servers probably too (depending to triggerCharacter implementation)

For now I modified tests

@mickaelistria
Copy link
Contributor

If you can provide the necessary fix in LSP4E, it would be very welcome!

@mickaelistria
Copy link
Contributor

@zulus Is it OK to merge now? test are not failing with this latest patch. Is there anything else to consider?

@zulus
Copy link
Contributor Author

zulus commented Sep 11, 2023

With current lsp4e state I can’t do more. Should be merged

@mickaelistria mickaelistria merged commit 659a769 into eclipse-wildwebdeveloper:master Sep 11, 2023
3 of 4 checks passed
@mickaelistria
Copy link
Contributor

OK. I think what matter most is really to check that the LS works overall; the very particular workflow in the test isn't too important here.

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.

3 participants