Skip to content

Reuse SMART search integration fixture#5588

Open
mikaelweave wants to merge 7 commits into
mainfrom
smart-search-fixture-reuse
Open

Reuse SMART search integration fixture#5588
mikaelweave wants to merge 7 commits into
mainfrom
smart-search-fixture-reuse

Conversation

@mikaelweave
Copy link
Copy Markdown
Contributor

Summary

  • Isolates the SMART search integration-test changes from the e2e-parallelization experiment branch.
  • SMART search tests previously recreated fixture/seeding per test method; this reuses a shared search fixture context.
  • The shared context is keyed by FHIR version + data store and disposed at process exit because custom xUnit synthetic fixture instances do not have normal fixture lifetime management.
  • Mutation tests use unique IDs; update tests create then update the same unique ID.

Validation

  • Diff versus main includes only test\Microsoft.Health.Fhir.Shared.Tests.Integration\Features\Smart\SmartSearchTests.cs.
  • git diff --check and git diff --check origin/main...HEAD passed.
  • dotnet build test\Microsoft.Health.Fhir.R4.Tests.Integration\Microsoft.Health.Fhir.R4.Tests.Integration.csproj -f net9.0 passed (Build succeeded in 149.5s).
  • Local R4 SQL SmartOnFhir shard on this split branch passed: 63/63 succeeded, 0 failed, test duration 68.5s, including build 156.5s.
  • Source experiment branch local R4 SQL SmartOnFhir result: 63/63 passed in about 60.1s test duration / 83.5s including build.

mikaelweave and others added 2 commits May 27, 2026 08:52
Log SMART integration fixture setup, resource upsert, and search-service timings so the PR pipeline can identify whether the slow SmartSearch shard is dominated by setup or specific query patterns.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Seed SMART integration data once per version and data store so SmartOnFhir tests avoid rebuilding the expensive storage fixture for every test method. Use unique IDs for mutation tests while preserving update semantics, and dispose cached fixtures when the test process exits.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.34%. Comparing base (ee554e9) to head (3828873).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #5588   +/-   ##
=======================================
  Coverage   77.33%   77.34%           
=======================================
  Files         996      996           
  Lines       36534    36539    +5     
  Branches     5538     5539    +1     
=======================================
+ Hits        28255    28260    +5     
+ Misses       6912     6908    -4     
- Partials     1367     1371    +4     

see 9 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Remove process-lifetime fixture caching and diagnostic timing wrappers while keeping shared SMART test data setup under xUnit fixture lifetime.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move test setup out of the no-op async lifecycle and inline the one-line upsert wrapper.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mikaelweave mikaelweave added this to the FY26\Q4\2Wk\2Wk24 milestone May 27, 2026
@mikaelweave mikaelweave added Open source This change is only relevant to the OSS code or release. No-ADR ADR not needed No-PaaS-breaking-change labels May 27, 2026
@mikaelweave mikaelweave marked this pull request as ready for review May 27, 2026 21:44
@mikaelweave mikaelweave requested a review from a team as a code owner May 27, 2026 21:44
…portunity'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Assert.DoesNotContain(results.Results, r => r.Resource.ResourceTypeName == "Practitioner");
}

public sealed class SmartSearchSharedFixture : IAsyncLifetime
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems like we usually put fixtures in their own file, rather than with the test cases. This is a suggestion, just to match the rest of the code base.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…o use Where'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Build No-ADR ADR not needed No-PaaS-breaking-change Open source This change is only relevant to the OSS code or release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants