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

[CI] Run display tests with both xml and json rpc api #11506

Merged
merged 7 commits into from
Jan 25, 2024

Conversation

kLabz
Copy link
Contributor

@kLabz kLabz commented Jan 24, 2024

... when available. Still need to migrate module-symbols to json rpc.

I made a bit of a mess while making sure json rpc api was giving the results expected by tests written for xml api. Should be possible to clean it up quite a bit when actually removing xml api.

One test is failing with json rpc, and it seems like this is because #6341 has been handled on xml api but not on json rpc? cc @Simn

@Simn
Copy link
Member

Simn commented Jan 25, 2024

I think that test is simply invalid for the new protocol because it reports everything and tells the IDE how to qualify identifiers in case of conflict.

@kLabz
Copy link
Contributor Author

kLabz commented Jan 25, 2024

Hmm, fair enough. Will update the test then.

@kLabz kLabz merged commit be88d42 into development Jan 25, 2024
122 checks passed
0b1kn00b pushed a commit to 0b1kn00b/haxe that referenced this pull request Jan 25, 2024
…#11506)

* [tests] prepare for json rpc vs xml display tests

* [tests] add display test for static field completion

Also run again a couple tests after caching type

* [tests] display test with both xml and json rpc api

* [ci] run both xml and jsonrpc display tests

* [display] send completion error when no results

(same as xml display api)

* [tests] don't rely on result.result == null on completion error

* [tests] only run Toplevel.testDuplicates for xml display
@kLabz kLabz deleted the ci/jsonprc-display-tests branch February 22, 2024 15:35
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.

2 participants