-
Notifications
You must be signed in to change notification settings - Fork 774
[READY] Update JDT to 0.40.0 #1266
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much !
Reviewed 4 of 4 files at r1.
Reviewable status: 1 of 2 LGTMs obtained (waiting on @bstaletic)
ycmd/tests/java/get_completions_test.py, line 664 at r1 (raw file):
CompletionEntryMatcher( 'InputStreamReader', None, { 'kind': 'Class', 'menu_text': 'InputStreamReader - jdk.internal.jline.internal',
This seems... odd. Why is it returning suggestions for jdk.internal
. That seems like a jdk.ls bug to me!
ycmd/tests/java/subcommands_test.py, line 1003 at r1 (raw file):
Quoted 4 lines of code…
# Again, this produces quite a lot of fussy little changes (that # actually lead to broken code, but we can't really help that), and # having them in this test would just be brittle without proving # anything about our code
not sure we need the whole of this comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 LGTMs obtained (and 1 stale) (waiting on @puremourning)
ycmd/tests/java/get_completions_test.py, line 664 at r1 (raw file):
Previously, puremourning (Ben Jackson) wrote…
This seems... odd. Why is it returning suggestions for
jdk.internal
. That seems like a jdk.ls bug to me!
How would I know why it returns jdk.internal
.
Check the logs... it's even more nuts!
'detailed_info': 'InitialServerRequestDispatcher - com.sun.corba.se.spi.protocol\n\n',
And not only that, but it returns just java.io
and that Sun thing... This is exactly the behaviour I saw on my laptop on 0.38.0.
I have no idea how to solve this, except by adding com.sun.cobra
blah blah blah... but that's fixing the consequences, not the source of the problem.
ycmd/tests/java/subcommands_test.py, line 1003 at r1 (raw file):
Previously, puremourning (Ben Jackson) wrote…
# Again, this produces quite a lot of fussy little changes (that # actually lead to broken code, but we can't really help that), and # having them in this test would just be brittle without proving # anything about our code
not sure we need the whole of this comment.
Copy/paste from some other place. Removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 LGTMs obtained (and 1 stale) (waiting on @puremourning)
ycmd/tests/java/get_completions_test.py, line 664 at r1 (raw file):
Previously, bstaletic (Boris Staletic) wrote…
How would I know why it returns
jdk.internal
.Check the logs... it's even more nuts!
'detailed_info': 'InitialServerRequestDispatcher - com.sun.corba.se.spi.protocol\n\n',
And not only that, but it returns just
java.io
and that Sun thing... This is exactly the behaviour I saw on my laptop on 0.38.0.I have no idea how to solve this, except by adding
com.sun.cobra
blah blah blah... but that's fixing the consequences, not the source of the problem.
shrug, probably not our bug whatever it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r2.
Reviewable status: 1 of 2 LGTMs obtained
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 of 2 LGTMs obtained (waiting on @bstaletic)
ycmd/tests/java/get_completions_test.py, line 664 at r1 (raw file):
Previously, puremourning (Ben Jackson) wrote…
shrug, probably not our bug whatever it is.
But how do I make tests pass?
I'm guessing the difference in response could be because of slightly different jdk versions. Should I just try to satisfy Azure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 LGTMs obtained (and 1 stale) (waiting on @puremourning)
ycmd/tests/java/get_completions_test.py, line 664 at r1 (raw file):
Previously, bstaletic (Boris Staletic) wrote…
But how do I make tests pass?
I'm guessing the difference in response could be because of slightly different jdk versions. Should I just try to satisfy Azure?
Okay, my stupid! Reverted to has_item()
. This test should pass now.
10988f4
to
a8d262b
Compare
This comment has been minimized.
This comment has been minimized.
Codecov Report
@@ Coverage Diff @@
## master #1266 +/- ##
=========================================
+ Coverage 96.94% 97.15% +0.2%
=========================================
Files 96 96
Lines 7375 7383 +8
Branches 153 153
=========================================
+ Hits 7150 7173 +23
+ Misses 184 169 -15
Partials 41 41 |
f6221ec
to
b61ebc5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed a flake8 bug, so this should be ready now.
Reviewable status: 0 of 2 LGTMs obtained (and 1 stale) (waiting on @puremourning)
ok let's rebase this one too now, shall we ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebased, but the issue with this one is that jdt on python2 on Windows doesn't seem to shut down, so the tests hang indefinitely.
Reviewable status: 0 of 2 LGTMs obtained (and 1 stale) (waiting on @puremourning)
904abc1
to
2430209
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finally green!
Reviewable status: 0 of 2 LGTMs obtained (and 1 stale) (waiting on @puremourning)
ycmd/tests/test_utils.py, line 453 at r3 (raw file):
def PollForMessages( app, request_data, timeout = 60 ):
I changed this default timeout to 60 because Poll_Diagnostics_ProjectWide_Eclipse_test
was quite flaky.
Is it okay to change the default or would it be better to change this in one java test that is actually flaky?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 LGTMs obtained (and 1 stale) (waiting on @puremourning)
ycmd/tests/test_utils.py, line 453 at r3 (raw file):
Previously, bstaletic (Boris Staletic) wrote…
I changed this default timeout to 60 because
Poll_Diagnostics_ProjectWide_Eclipse_test
was quite flaky.Is it okay to change the default or would it be better to change this in one java test that is actually flaky?
Default is fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 7 files at r3.
Reviewable status: 0 of 2 LGTMs obtained (and 1 stale) (waiting on @puremourning)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 of 2 LGTMs obtained (waiting on @puremourning)
Thanks for sending a PR! |
ycm-core/ycmd#1275 Remap python GoTo* commands ycm-core/ycmd#1274 Migrate to Omnisharp-Roslyn ycm-core/ycmd#1239 Error ID in diagnostic messages ycm-core/ycmd#1267 Clangd extra conf support ycm-core/ycmd#1266 JDT update to 0.40.0 ycm-core/ycmd#1245 Generic (pluggable) LSP completer support ycm-core/ycmd#1263 Support JDT extensions ycm-core/ycmd#1261 Typescript update ycm-core/ycmd#1264 [vimspector](/puremourning/vimspector) configuration ycm-core/ycmd#1262 Multi-user installation ycm-core/ycmd#1243 Migrate to GOPLS ycm-core/ycmd#1249 Support MSVC 16 (VS2019) ycm-core/ycmd#1224 Migrate to RLS
ycm-core/ycmd#1275 Remap python GoTo* commands ycm-core/ycmd#1274 Migrate to Omnisharp-Roslyn ycm-core/ycmd#1239 Error ID in diagnostic messages ycm-core/ycmd#1267 Clangd extra conf support ycm-core/ycmd#1266 JDT update to 0.40.0 ycm-core/ycmd#1245 Generic (pluggable) LSP completer support ycm-core/ycmd#1263 Support JDT extensions ycm-core/ycmd#1261 Typescript update ycm-core/ycmd#1264 [vimspector](/puremourning/vimspector) configuration ycm-core/ycmd#1262 Multi-user installation ycm-core/ycmd#1243 Migrate to GOPLS ycm-core/ycmd#1249 Support MSVC 16 (VS2019) ycm-core/ycmd#1224 Migrate to RLS
ycm-core/ycmd#1275 Remap python GoTo* commands ycm-core/ycmd#1274 Migrate to Omnisharp-Roslyn ycm-core/ycmd#1239 Error ID in diagnostic messages ycm-core/ycmd#1267 Clangd extra conf support ycm-core/ycmd#1266 JDT update to 0.40.0 ycm-core/ycmd#1245 Generic (pluggable) LSP completer support ycm-core/ycmd#1263 Support JDT extensions ycm-core/ycmd#1261 Typescript update ycm-core/ycmd#1264 [vimspector](/puremourning/vimspector) configuration ycm-core/ycmd#1262 Multi-user installation ycm-core/ycmd#1243 Migrate to GOPLS ycm-core/ycmd#1249 Support MSVC 16 (VS2019) ycm-core/ycmd#1224 Migrate to RLS
ycm-core/ycmd#1275 Remap python GoTo* commands ycm-core/ycmd#1274 Migrate to Omnisharp-Roslyn ycm-core/ycmd#1239 Error ID in diagnostic messages ycm-core/ycmd#1267 Clangd extra conf support ycm-core/ycmd#1266 JDT update to 0.40.0 ycm-core/ycmd#1245 Generic (pluggable) LSP completer support ycm-core/ycmd#1263 Support JDT extensions ycm-core/ycmd#1261 Typescript update ycm-core/ycmd#1264 [vimspector](/puremourning/vimspector) configuration ycm-core/ycmd#1262 Multi-user installation ycm-core/ycmd#1243 Migrate to GOPLS ycm-core/ycmd#1249 Support MSVC 16 (VS2019) ycm-core/ycmd#1224 Migrate to RLS
ycm-core/ycmd#1275 Remap python GoTo* commands ycm-core/ycmd#1274 Migrate to Omnisharp-Roslyn ycm-core/ycmd#1239 Error ID in diagnostic messages ycm-core/ycmd#1267 Clangd extra conf support ycm-core/ycmd#1266 JDT update to 0.40.0 ycm-core/ycmd#1245 Generic (pluggable) LSP completer support ycm-core/ycmd#1263 Support JDT extensions ycm-core/ycmd#1261 Typescript update ycm-core/ycmd#1264 [vimspector](/puremourning/vimspector) configuration ycm-core/ycmd#1262 Multi-user installation ycm-core/ycmd#1243 Migrate to GOPLS ycm-core/ycmd#1249 Support MSVC 16 (VS2019) ycm-core/ycmd#1224 Migrate to RLS
A few significant changes:
This change is