-
Notifications
You must be signed in to change notification settings - Fork 567
Add tests to improve R4.Core tests coverage #5260
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
base: main
Are you sure you want to change the base?
Conversation
| { | ||
| // Arrange | ||
| const string operationName = "export"; | ||
| var cancellationTokenSource = new CancellationTokenSource(); |
Check warning
Code scanning / CodeQL
Missing Dispose call on local IDisposable Warning
|
|
||
| // Assert | ||
| Assert.NotNull(result); | ||
| var resultPatient = result.ToPoco<Patient>(); |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning
resultPatient
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
To fix the useless assignment, remove the declaration and assignment to the resultPatient variable on line 254. Since the value is never used, this can be deleted without affecting program logic or test coverage. You should update only the portion with the assignment so that the code reads smoothly without the variable. No imports or additional code are required.
-
Copy modified line R254
| @@ -251,7 +251,7 @@ | ||
|
|
||
| // Assert | ||
| Assert.NotNull(result); | ||
| var resultPatient = result.ToPoco<Patient>(); | ||
| // Removed useless assignment to resultPatient | ||
| Assert.Equal("patient1", matchingPatient.Id); | ||
| } | ||
|
|
| foreach (var type in new[] { "ValueSet", "CodeSystem", "StructureDefinition" }) | ||
| { | ||
| if (type != resourceType) | ||
| { | ||
| _searchService.SearchAsync( | ||
| type, | ||
| Arg.Any<List<Tuple<string, string>>>(), | ||
| Arg.Any<CancellationToken>()) | ||
| .Returns(new SearchResult(new List<SearchResultEntry>(), null, null, new List<Tuple<string, string>>())); | ||
| } | ||
| } |
Check notice
Code scanning / CodeQL
Missed opportunity to use Where Note
implicitly filters its target sequence
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
To address the issue, the foreach loop in SetupSearchServiceWithResults should iterate only over elements where type != resourceType, utilizing LINQ's .Where(...) method for clarity and intent. The if (type != resourceType) guard inside the loop becomes superfluous and should be removed. There is no change to existing functionality—just a refactor to express filtering more idiomatically. Only the loop over the array on line 322–332 in SetupSearchServiceWithResults needs to be updated. Since System.Linq is already imported (line 8), no imports are required.
-
Copy modified line R322 -
Copy modified lines R324-R328
| @@ -319,16 +319,13 @@ | ||
| .Returns(searchResult); | ||
|
|
||
| // Setup for other resource types to return empty | ||
| foreach (var type in new[] { "ValueSet", "CodeSystem", "StructureDefinition" }) | ||
| foreach (var type in new[] { "ValueSet", "CodeSystem", "StructureDefinition" }.Where(type => type != resourceType)) | ||
| { | ||
| if (type != resourceType) | ||
| { | ||
| _searchService.SearchAsync( | ||
| type, | ||
| Arg.Any<List<Tuple<string, string>>>(), | ||
| Arg.Any<CancellationToken>()) | ||
| .Returns(new SearchResult(new List<SearchResultEntry>(), null, null, new List<Tuple<string, string>>())); | ||
| } | ||
| _searchService.SearchAsync( | ||
| type, | ||
| Arg.Any<List<Tuple<string, string>>>(), | ||
| Arg.Any<CancellationToken>()) | ||
| .Returns(new SearchResult(new List<SearchResultEntry>(), null, null, new List<Tuple<string, string>>())); | ||
| } | ||
| } | ||
|
|
| var emptyResult = new SearchResult(Enumerable.Empty<SearchResultEntry>(), null, null, new Tuple<string, string>[0]); | ||
| _searchService.SearchAsync(Arg.Any<SearchOptions>(), CancellationToken.None).Returns(emptyResult); | ||
|
|
||
| SearchResult actualResult = await _patientEverythingService.SearchAsync("123", null, null, null, null, null, CancellationToken.None); |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning
actualResult
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
To fix the problem, simply remove the assignment to the actualResult variable. In the context of an await on an asynchronous function, if the returned value is not needed, just perform the call with await and omit assigning its result. This removes the useless variable and its assignment, while still triggering any required behavior of the awaited function.
Specifically, edit src/Microsoft.Health.Fhir.Shared.Core.UnitTests/Features/Operations/Everything/PatientEverythingServiceTests.cs, and in the method for the test GivenPhase0WithNoResults_WhenSearch_ThenProceedsToPhase1, remove the entire assignment statement on line 195 and replace it with just the awaited call:
await _patientEverythingService.SearchAsync("123", null, null, null, null, null, CancellationToken.None);No additional imports, method definitions, or variable declarations are necessary.
-
Copy modified line R195
| @@ -192,7 +192,7 @@ | ||
| var emptyResult = new SearchResult(Enumerable.Empty<SearchResultEntry>(), null, null, new Tuple<string, string>[0]); | ||
| _searchService.SearchAsync(Arg.Any<SearchOptions>(), CancellationToken.None).Returns(emptyResult); | ||
|
|
||
| SearchResult actualResult = await _patientEverythingService.SearchAsync("123", null, null, null, null, null, CancellationToken.None); | ||
| await _patientEverythingService.SearchAsync("123", null, null, null, null, null, CancellationToken.None); | ||
|
|
||
| // Should have called search multiple times (phase 0, then phase 1/2) | ||
| await _searchService.Received().SearchAsync(Arg.Any<SearchOptions>(), Arg.Any<CancellationToken>()); |
| var start = PartialDateTime.Parse("2020-01-01"); | ||
| var end = PartialDateTime.Parse("2020-12-31"); | ||
|
|
||
| SearchResult actualResult = await _patientEverythingService.SearchAsync("123", start, end, null, null, null, CancellationToken.None); |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning
actualResult
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
To fix this problem, remove the assignment of the result of _patientEverythingService.SearchAsync(...) to the unused local variable actualResult, and simply await the call as a statement. This change should be made only within the GivenPhase1WithDateRange_WhenSearch_ThenSearchCompartmentWithDateIsCalled test method, at line 216. No other changes are required, as there is no use of actualResult later in the method. This will improve code clarity and eliminate the useless assignment flagged by CodeQL.
-
Copy modified line R216
| @@ -213,7 +213,7 @@ | ||
| var start = PartialDateTime.Parse("2020-01-01"); | ||
| var end = PartialDateTime.Parse("2020-12-31"); | ||
|
|
||
| SearchResult actualResult = await _patientEverythingService.SearchAsync("123", start, end, null, null, null, CancellationToken.None); | ||
| await _patientEverythingService.SearchAsync("123", start, end, null, null, null, CancellationToken.None); | ||
|
|
||
| // Should have searched with date parameters | ||
| await _searchService.Received().SearchAsync(Arg.Any<SearchOptions>(), CancellationToken.None); |
| var emptyResult = new SearchResult(Enumerable.Empty<SearchResultEntry>(), null, null, new Tuple<string, string>[0]); | ||
| _searchService.SearchAsync(Arg.Any<SearchOptions>(), CancellationToken.None).Returns(emptyResult); | ||
|
|
||
| SearchResult actualResult = await _patientEverythingService.SearchAsync("123", null, null, null, null, null, CancellationToken.None); |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning
actualResult
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
To address this issue, simply remove the assignment to the unused variable actualResult, but keep the await call to _patientEverythingService.SearchAsync(...) to ensure the side effects and possible exceptions are still observed (as may be needed for the test). Replace
SearchResult actualResult = await _patientEverythingService.SearchAsync(...);with
await _patientEverythingService.SearchAsync(...);This preserves all current functionality but eliminates the useless assignment. No additional imports, definitions, or method changes are needed.
-
Copy modified line R234
| @@ -231,7 +231,7 @@ | ||
| var emptyResult = new SearchResult(Enumerable.Empty<SearchResultEntry>(), null, null, new Tuple<string, string>[0]); | ||
| _searchService.SearchAsync(Arg.Any<SearchOptions>(), CancellationToken.None).Returns(emptyResult); | ||
|
|
||
| SearchResult actualResult = await _patientEverythingService.SearchAsync("123", null, null, null, null, null, CancellationToken.None); | ||
| await _patientEverythingService.SearchAsync("123", null, null, null, null, null, CancellationToken.None); | ||
|
|
||
| // Should proceed through phases | ||
| await _searchService.Received().SearchAsync(Arg.Any<SearchOptions>(), Arg.Any<CancellationToken>()); |
| var patient = Samples.GetDefaultPatient(); | ||
| var coverage = Samples.GetDefaultCoverage(); | ||
| var request = new MemberMatchRequest(coverage, patient); | ||
| var cts = new CancellationTokenSource(); |
Check warning
Code scanning / CodeQL
Missing Dispose call on local IDisposable Warning
Description
Add tests to improve R4.Core tests coverage
Related issues
Addresses 174931
Testing
Describe how this change was tested.
FHIR Team Checklist
Semver Change (docs)
Patch|Skip|Feature|Breaking (reason)