updates unit tests and adds github action ci support#113
Merged
brynrhodes merged 3 commits intomasterfrom Mar 11, 2026
Merged
Conversation
adds github action ci support
|
Formatting check succeeded! |
Coverage Report
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #113 +/- ##
============================================
+ Coverage 50.29% 55.21% +4.92%
- Complexity 278 299 +21
============================================
Files 46 46
Lines 1543 1543
Branches 189 189
============================================
+ Hits 776 852 +76
+ Misses 703 618 -85
- Partials 64 73 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
brynrhodes
approved these changes
Mar 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #111
PR: Improve unit test coverage, remove Spring from test suite
Branch: feature/improve-unit-tests
Why remove Spring Boot from tests?
Two reasons, one tactical and one strategic.
Tactical:
@SpringBootTestspins up the full application context for eachtest class. That's unnecessary overhead when you only need to test a single
provider or service. Direct instantiation is faster and makes dependencies
explicit — you can see exactly what a class needs without Spring's wiring hiding
it.
Strategic: The ongoing Kotlin migration (Phase 4) deletes
ServerConfigandPluginConfigand replaces Spring DI with manual wiring inmain.kt. OnceSpring is gone from the production code,
@SpringBootTesttests can't work atall. Migrating the tests now means they won't break when Phase 4 lands — and it
unblocks deleting
TestConfig.javarather than carrying it forward as deadweight.
HoverProviderTestwas already written the right way (direct instantiation, noSpring).
LanguageServerTestandDiagnosticsServiceTestwere the twostragglers.
Overview
Removes the last two Spring Boot test dependencies in
ls/server/, adds a newFormattingProvidertest class, and expands coverage forDiagnosticsService,HoverProvider, andExpressionTrackBackVisitor. Net result: 163 → 165 tests,zero Spring annotations remaining in
ls/server/test sources.Files Deleted
LanguageServerTest.javaAll three hover tests were identical to the existing
HoverProviderTesttests.The fourth test (
handshake) only assertedassertNotNull(server). The classcomment itself said "just sketching out some tests here." Removing it eliminates
duplication and a Spring dependency with no coverage loss.
config/TestConfig.javaOnly existed to wire a
TestContentServicebean for the two Spring-based tests.With both of those tests migrated to direct instantiation,
TestConfighas noremaining callers and can be deleted.
Files Modified
DiagnosticsServiceTest.java@SpringBootTest(classes = {TestConfig.class})and@Autowired@BeforeAllwith direct instantiation:ContentService cs = new TestContentService();
CqlCompilationManager mgr = new CqlCompilationManager(cs, new CompilerOptionsManager(cs), new IgContextManager(cs));
diagnosticsService = new DiagnosticsService(CompletableFuture.completedFuture(mock(LanguageClient.class)), mgr, cs);
missingIncludetest (unchanged)validCql_noErrors: lintsTwo.cql, asserts result contains the URI keywith an empty diagnostic set
syntaxError_returnsDiagnostic: lints newSyntaxError.cqlfixture,asserts at least one diagnostic with severity ERROR
provider/HoverProviderTest.javahoverOnLibraryRef: position (5, 8) is 'O' inOne."One"on line 5(0-indexed) of Two.cql — expects
System.Integer(library-qualified referenceresolves to the expression type)
hoverOnDefineName: position (4, 8) is 'T' insidedefine "Two":—ExpressionDef TrackBack covers the define header, expects
System.Integervisitor/ExpressionTrackBackVisitorTest.javapositionInExpressionRef_returnsExpressionDef: TrackBack (12, 5, 12, 25)covers the
"ObservationRetrieve"ExpressionRef inside theObservationReferencedefine — asserts result is an ExpressionDef
positionAtExpressionBoundary_returnsExpressionDef: TrackBack (15, 5, 15, 5)at the start character of
Patient.birthDatein thePropertyAccessdefine —asserts result is an ExpressionDef
Files Created
provider/FormattingProviderTest.java(new — was entirely untested)new FormattingProvider(new TestContentService())format_validCql_returnsOneEdit: formatsTwo.cql, asserts exactly one TextEditreturned, range starts at (0, 0), and
newTextis non-blankformat_syntaxError_throwsIllegalArgument: formatsSyntaxError.cql, assertsIllegalArgumentExceptionis thrown (formatter rejects CQL with parse errors)resources/.../SyntaxError.cql(new fixture)library SyntaxError version '1.0.0'
Used by both
DiagnosticsServiceTest.syntaxError_returnsDiagnosticandFormattingProviderTest.format_syntaxError_throwsIllegalArgument.Test Count
Before: 163 (includes 4 Spring-based tests that are now removed)
After: 165
Removed: 4 (LanguageServerTest: handshake, hoverInt, hoverNothing, hoverList)
Added: 6 (validCql_noErrors, syntaxError_returnsDiagnostic, hoverOnLibraryRef,
hoverOnDefineName, positionInExpressionRef_returnsExpressionDef,
positionAtExpressionBoundary_returnsExpressionDef)
Added: 2 (FormattingProviderTest: format_validCql_returnsOneEdit,
format_syntaxError_throwsIllegalArgument)
Net: +2
Verification