Repository navigation
test(lsp): wait on observable conditions instead of fixed sleeps - #325
Conversation
Vulnetix config planManager
0 to create, 1 to update, 0 to delete, 0 unchanged |
0x73746F66
left a comment
0x73746F66
left a comment
There was a problem hiding this comment.
Reuse review: test-only. The condition waits (waitForMessage, waitForRules, waitForWarm) extend the existing lockedBuffer/testClient harness already used by the LSP stdio and conformance tests rather than adding a second harness; there is no shared polling helper in internal/testutils to reuse. No changes needed.
0x73746F66
left a comment
0x73746F66
left a comment
There was a problem hiding this comment.
Reuse review: no reinvented Vulnetix patterns found. The change stays inside the package it covers, uses stdlib/testify helpers already used by the repo's tests, and adds no new client, auth, config-schema or shared-package duplicate (checked internal/testutils and sibling repos). Rebased onto current main locally: go vet and go test pass for every touched package. The earlier red checks (CodeQL Analyze, Install a release with verified provenance, Lockfile) were GitHub API rate-limit failures, not code failures; this will be re-run on a rebased head before merge.
- waitForWarm polls vulnetix/serverInfo until the warm-up has loaded the rules (stored together with the workspace index) instead of sleeping 3s. - The outside-workspace test opens an in-workspace file after the outside one and waits for its diagnostics as a barrier, instead of sleeping 500ms and hoping a late publish would have arrived. - TestNoStdoutPollution reads stdout through a locked buffer, waits for the rules before opening the document, then waits for its publishDiagnostics and for the shutdown response, instead of sleeping 6s and 300ms. Closes #221
f63da9e to
97fca6c
Compare
Vulnetix config planManager
0 to create, 1 to update, 0 to delete, 0 unchanged |
Closes #221
What
internal/lsp/conformance_test.gowaitForWarm(t, c)pollsvulnetix/serverInfowithrequire.EventuallyuntilrulesEmbedded > 0. The warm-up stores the rules and the workspace index together, which is exactly when adidOpenis analysed. Was a fixed 3s sleep.TestDocumentsOutsideTheWorkspaceAreNotScanned: after opening the outside file, opens a vulnerable file inside the workspace and waits for its diagnostics (waitForPublish), then asserts the outside file has none. Was a 500ms sleep, which could pass before a late publish arrived.cmd/lsp_stdio_test.go(TestNoStdoutPollution)initialize/initializedit asksvulnetix/serverInfountil the rules are loaded (a document opened during warm-up is not analysed until its next change), then opens/changes/saves the document and waits for itspublishDiagnostics, then waits for theshutdownresponse beforeexit. Was 6s + 300ms of sleeps. The final strict framing check over all of stdout is unchanged.Reuses the package's existing
waitForPublish,require.EventuallyandparseFrames; no production change.Not changed
TestWorkspaceScanCanBeCancelledkeeps its 50ms pause beforecancel(). The server emits nothing when a workspace scan starts, so there is no condition to wait on, and the assertion (a cancelled call returns an error) holds whichever side of the start the cancel lands. Adding a progress signal to the server just for this test is out of scope here.Verified
go test -run 'TestDidOpenPublishesDiagnostics|TestClosingADocument|TestDocumentsOutsideTheWorkspace|TestWorkspaceScanCanBeCancelled' ./internal/lsp/: pass (outside-workspace test 0.6s, was >3.5s).go test -run TestNoStdoutPollution ./cmd/: pass, 5 clean frames parsed.go vet,gofmtclean.