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

Improve hls class plugin test #4059

Merged
merged 1 commit into from
Feb 8, 2024

Conversation

soulomoon
Copy link
Collaborator

@soulomoon soulomoon commented Feb 7, 2024

I don't think we need to pass in the source to match the result, since its result is throwed away.
switching from waitForDiagnosticsFromSource to waitForDiagnosticsFrom improve the test speed by magnitude.

before
All 29 tests passed (88.71s)
after
All 29 tests passed (5.74s)

@soulomoon soulomoon force-pushed the improve-hls-class-plugin-test branch from 94cf26e to 5bee014 Compare February 7, 2024 19:52
@soulomoon soulomoon marked this pull request as ready for review February 7, 2024 19:53
@soulomoon soulomoon marked this pull request as draft February 7, 2024 20:41
@soulomoon

This comment was marked as off-topic.

@soulomoon soulomoon force-pushed the improve-hls-class-plugin-test branch from 708de54 to 5bee014 Compare February 7, 2024 21:11
@soulomoon soulomoon marked this pull request as ready for review February 7, 2024 21:14
@@ -124,7 +124,7 @@ codeLensTests = testGroup
, testCase "Do not construct error action!, Ticket3942one" $ do
runSessionWithServer def classPlugin testDataDir $ do
doc <- openDoc "Ticket3942one.hs" "haskell"
_ <- waitForDiagnosticsFromSource doc (T.unpack sourceTypecheck)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function is rather weird. I don't know why it isn't just a slight variant of waitForDiagnostics that adds an extra predicate...

Copy link
Collaborator

@michaelpj michaelpj left a comment

Choose a reason for hiding this comment

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

Should we just get rid of waitForDiagnosticsWithSource entirely? It seems like it's going to introduce unconditional waits everywhere, which seems pretty bad!

@soulomoon
Copy link
Collaborator Author

soulomoon commented Feb 8, 2024

waitForDiagnosticsWithSource

This one has a timeout, and something I am not so sure about. It seems some test rely on the timeout.
I tried a naive fix but did not success here(mainly because of those test that rely on the timeout), It seems rather complex.
Maybe we can open a seperate issue about getting rid of(or fixing it to perform better) waitForDiagnosticsWithSource(and those test that have unconditional waiting problem)

@michaelpj
Copy link
Collaborator

Okay, well maybe let's at least note it down as something to investigate in #3736

@michaelpj michaelpj merged commit 03efae6 into haskell:master Feb 8, 2024
61 checks passed
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.

None yet

2 participants