Skip to content

Adds Create E2E test for all R4 Resource types #5019

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Adds Create E2E test for all R4 Resource types #5019

wants to merge 1 commit into from

Conversation

brendankowitz
Copy link
Member

@brendankowitz brendankowitz commented Jun 9, 2025 •
edited by azure-boards bot
Loading

Description

This pull request introduces a new shared project, Microsoft.Health.Fhir.Shared.Tests.Fakes, which provides utilities and extensions for testing FHIR resources. It also integrates the new project into the solution and updates existing tests to use these utilities.

Updates to tests:

Dependency updates:

Related issues

This is an alternative approach for AB#143833

Testing

Describe how this change was tested.

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)

@brendankowitz brendankowitz force-pushed the personal/bkowitz/fhir-faker branch 2 times, most recently from 52968c7 to 40d613f Compare June 9, 2025 19:09
@brendankowitz brendankowitz force-pushed the personal/bkowitz/fhir-faker branch from 40d613f to f65c3a6 Compare June 9, 2025 20:35
github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 9, 2025
View reviewed changes
src/Microsoft.Health.Fhir.Shared.Tests.Fakes/FhirFakesFactory.cs
Comment on lines +366 to +369
catch (Exception e)
{
Trace.WriteLine("Unable to set property " + prop.Name + " on " + obj.GetType().Name + ": " + e.Message);
}

Check notice

Code scanning / CodeQL

Generic catch clause Note

Generic catch clause.

Copilot Autofix

AI 25 days ago

To fix the issue, replace the generic catch (Exception e) block with specific exception handling. The most likely exceptions that could occur in this context are:

  1. ArgumentException or its derived types (e.g., ArgumentNullException, ArgumentOutOfRangeException) if the property value is invalid.
  2. TargetInvocationException if the property setter throws an exception.
  3. InvalidOperationException if the property cannot be set due to some runtime condition.

The fix involves:

  • Catching these specific exceptions individually.
  • Logging the exception details for debugging purposes.
  • Allowing other exceptions to propagate by not catching them.
Suggested changeset 1
src/Microsoft.Health.Fhir.Shared.Tests.Fakes/FhirFakesFactory.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/Microsoft.Health.Fhir.Shared.Tests.Fakes/FhirFakesFactory.cs b/src/Microsoft.Health.Fhir.Shared.Tests.Fakes/FhirFakesFactory.cs
--- a/src/Microsoft.Health.Fhir.Shared.Tests.Fakes/FhirFakesFactory.cs
+++ b/src/Microsoft.Health.Fhir.Shared.Tests.Fakes/FhirFakesFactory.cs
@@ -365,5 +365,13 @@
                 }
-                catch (Exception e)
+                catch (ArgumentException e)
                 {
-                    Trace.WriteLine("Unable to set property " + prop.Name + " on " + obj.GetType().Name + ": " + e.Message);
+                    Trace.WriteLine("Argument exception while setting property " + prop.Name + " on " + obj.GetType().Name + ": " + e.Message);
+                }
+                catch (TargetInvocationException e)
+                {
+                    Trace.WriteLine("Invocation exception while setting property " + prop.Name + " on " + obj.GetType().Name + ": " + e.Message);
+                }
+                catch (InvalidOperationException e)
+                {
+                    Trace.WriteLine("Invalid operation while setting property " + prop.Name + " on " + obj.GetType().Name + ": " + e.Message);
                 }
EOF
@@ -365,5 +365,13 @@
}
catch (Exception e)
catch (ArgumentException e)
{
Trace.WriteLine("Unable to set property " + prop.Name + " on " + obj.GetType().Name + ": " + e.Message);
Trace.WriteLine("Argument exception while setting property " + prop.Name + " on " + obj.GetType().Name + ": " + e.Message);
}
catch (TargetInvocationException e)
{
Trace.WriteLine("Invocation exception while setting property " + prop.Name + " on " + obj.GetType().Name + ": " + e.Message);
}
catch (InvalidOperationException e)
{
Trace.WriteLine("Invalid operation while setting property " + prop.Name + " on " + obj.GetType().Name + ": " + e.Message);
}
Copilot is powered by AI and may make mistakes. Always verify output.
@brendankowitz brendankowitz marked this pull request as ready for review June 11, 2025 18:15
@brendankowitz brendankowitz requested a review from a team as a code owner June 11, 2025 18:15
@brendankowitz brendankowitz requested a review from Copilot June 11, 2025 18:15
Copilot
Copilot AI reviewed Jun 11, 2025
View reviewed changes
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new shared test fakes project for generating and linking FHIR resources, integrates it into the solution, and updates existing E2E create tests to use the new utilities and output logging.

  • Introduce Microsoft.Health.Fhir.Shared.Tests.Fakes project containing FhirFakesFactory and FhirResourceExtensions
  • Update Shared and R4 CreateTests to use FhirFakesFactory and support ITestOutputHelper logging
  • Add Bogus package for fake data generation

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/Microsoft.Health.Fhir.Shared.Tests.E2E/Rest/CreateTests.cs Made CreateTests partial and injected ITestOutputHelper for logging
test/Microsoft.Health.Fhir.R4.Tests.E2E/Rest/CreateTests.cs Added R4-specific CreateTests using FhirFakesFactory
test/Microsoft.Health.Fhir.R4.Tests.E2E/Microsoft.Health.Fhir.R4.Tests.E2E.csproj Added Bogus package reference and imported shared fakes items
src/Microsoft.Health.Fhir.Shared.Tests.Fakes/ New shared test fakes project, including factory and extension methods
Directory.Packages.props Added central version for Bogus package
Comments suppressed due to low confidence (2)

src/Microsoft.Health.Fhir.Shared.Tests.Fakes/FhirResourceExtensions.cs:25

  • The new utility method has no corresponding unit tests. Consider adding tests to verify that LinkToPatient correctly sets patient references on various resource types.
public static T LinkToPatient<T>(this T resource, Patient patient)

test/Microsoft.Health.Fhir.R4.Tests.E2E/Rest/CreateTests.cs:37

  • ITestOutputHelper is used but not injected or defined in this class. Add a private ITestOutputHelper field and modify the constructor to accept and assign the output helper to enable logging.
_outputHelper.WriteLine(resourceJson);

@brendankowitz brendankowitz added this to the CY25Q2/2Wk06 milestone Jun 13, 2025
@brendankowitz brendankowitz added Enhancement-Test Enhancement on tests. Build Open source This change is only relevant to the OSS code or release. labels Jun 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Build Enhancement-Test Enhancement on tests. Open source This change is only relevant to the OSS code or release.
Projects
None yet
Milestone
CY25Q2/2Wk06
Development

Successfully merging this pull request may close these issues.
1 participant
@brendankowitz