Skip to content

Improve SqlServer project test coverage - part 2#5360

Merged
rbans96 merged 26 commits intomainfrom
personal/ribans/test-coverage-sqlServer-2
Feb 5, 2026
Merged

Improve SqlServer project test coverage - part 2#5360
rbans96 merged 26 commits intomainfrom
personal/ribans/test-coverage-sqlServer-2

Conversation

@rbans96
Copy link
Contributor

@rbans96 rbans96 commented Jan 29, 2026 •
edited
Loading

Description

Improve SqlServer project test coverage.

Related issues

Addresses 181809

Testing

image

FHIR Team Checklist

  • Update the title of the PR to be succinct and less than 65 characters
  • Add a milestone to the PR for the sprint that it is merged (i.e. add S47)
  • Tag the PR with the type of update: Bug, Build, Dependencies, Enhancement, New-Feature or Documentation
  • Tag the PR with Open source, Azure API for FHIR (CosmosDB or common code) or Azure Healthcare APIs (SQL or common code) to specify where this change is intended to be released.
  • Tag the PR with Schema Version backward compatible or Schema Version backward incompatible or Schema Version unchanged if this adds or updates Sql script which is/is not backward compatible with the code.
  • When changing or adding behavior, if your code modifies the system design or changes design assumptions, please create and include an ADR.
  • CI is green before merge Build Status
  • Review squash-merge requirements

Semver Change (docs)

Patch|Skip|Feature|Breaking (reason)

@rbans96 rbans96 requested a review from a team as a code owner January 29, 2026 23:54
@rbans96 rbans96 marked this pull request as draft January 29, 2026 23:54
@rbans96 rbans96 changed the title Improve SqlServer project test coverage - part 2 [Draft] Improve SqlServer project test coverage - part 2 Jan 29, 2026
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 30, 2026
View reviewed changes
Copy link
Contributor

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@rbans96
Copy link
Contributor Author

rbans96 commented Jan 30, 2026

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Jan 30, 2026

Azure Pipelines successfully started running 1 pipeline(s).

@rbans96
Copy link
Contributor Author

rbans96 commented Feb 3, 2026

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Feb 3, 2026

Azure Pipelines successfully started running 1 pipeline(s).

@rbans96 rbans96 added Enhancement-Test Enhancement on tests. Azure API for FHIR Label denotes that the issue or PR is relevant to the Azure API for FHIR No-ADR ADR not needed labels Feb 3, 2026
@rbans96 rbans96 added this to the CY25Q3/2Wk07 milestone Feb 3, 2026
@rbans96 rbans96 changed the title [Draft] Improve SqlServer project test coverage - part 2 Improve SqlServer project test coverage - part 2 Feb 3, 2026
@rbans96 rbans96 marked this pull request as ready for review February 4, 2026 17:59
@rbans96
Fix CodeQL warnings: null dereferences and unused variables
ea69cb0 
- SqlServerSearchServiceIntegrationTests: Remove unused 'observations' list, fix nullable dereference
- SqlQueueClientTests: Remove useless assignment to 'func' variable
- IncludesContinuationTokenTests: Add Assert.NotNull before accessing nullable .Value properties
- StringOverflowRewriterTests: Add null-forgiving operators after Assert.NotNull for 'as' casts
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 4, 2026
View reviewed changes
mikaelweave

This comment was marked as duplicate.

Sign in to view

@rbans96
Fix CodeQL null dereference warnings in StringOverflowRewriterTests
542ae1e 
Add null-forgiving operators (!) consistently after Assert.NotNull checks
to satisfy CodeQL static analysis for all variable accesses.
@rbans96
address feedback
d6ad82b
@rbans96
Copy link
Contributor Author

rbans96 commented Feb 5, 2026

Regarding the concurrent update scenario question:

Concurrent update tests already exist in a dedicated test file: SearchParameterOptimisticConcurrencyIntegrationTests.cs. This file contains tests for:

  • ✅ Updating with correct LastUpdated succeeds
  • ✅ Rapid concurrent updates with stale LastUpdated triggers the retry mechanism
  • ✅ The retry mechanism handles high contention scenarios (5 parallel updates)
  • ✅ Multiple consecutive stale updates still succeed with retry
  • ✅ Mixed new/existing parameter updates

The SqlServerSearchParameterStatusDataStoreTests.cs file focuses on the basic CRUD operations and SQL-specific features, while concurrency testing is handled separately in the optimistic concurrency test file.

@rbans96 rbans96 merged commit 9423b8e into main Feb 5, 2026
61 checks passed
@rbans96 rbans96 deleted the personal/ribans/test-coverage-sqlServer-2 branch February 5, 2026 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mikaelweave mikaelweave mikaelweave approved these changes

Assignees

No one assigned

Labels

Azure API for FHIR Label denotes that the issue or PR is relevant to the Azure API for FHIR Enhancement-Test Enhancement on tests. No-ADR ADR not needed No-PaaS-breaking-change

Projects

None yet

Milestone

CY25Q3/2Wk07

Development

Successfully merging this pull request may close these issues.

2 participants

@rbans96 @mikaelweave