Skip to content

Commit

Permalink
Fix: interprocess mutex cannot be async.
Browse files Browse the repository at this point in the history 
Add: unit tests comparing json output should compare by JObject, not string.
  • Loading branch information
@GinoCanessa
GinoCanessa committed Sep 4, 2024
1 parent 25f3fd3 commit 874cc2e
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 23 deletions.
48 changes: 32 additions & 16 deletions src/Microsoft.Health.Fhir.CodeGen.Tests/GenerationTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
using Microsoft.Health.Fhir.CodeGen.Models;
using Microsoft.Health.Fhir.CodeGen.Tests.Extensions;
using Microsoft.Health.Fhir.CodeGenCommon.Packaging;
using Newtonsoft.Json.Linq;
using Xunit.Abstractions;

namespace Microsoft.Health.Fhir.CodeGen.Tests;
Expand Down Expand Up @@ -45,7 +46,10 @@ public GenerationTestFixture()
/// <param name="existingPath"> Full pathname of the existing file.</param>
/// <param name="currentMS"> The current milliseconds.</param>
/// <param name="compareLinesDirectly">(Optional) True to compare lines directly, false to compare set of line hashes.</param>
internal static void CompareGeneration(string existingPath, MemoryStream currentMS, bool compareLinesDirectly = true)
internal static void CompareGeneration(
string existingPath,
MemoryStream currentMS,
bool compareLinesDirectly = true)
{
// make sure the MS is up to date and at the beginning
currentMS.Flush();
Expand Down Expand Up @@ -82,6 +86,20 @@ internal static void CompareGeneration(string existingPath, MemoryStream current
version = version.Substring(0, version.IndexOf('+'));
}

if (existingPath.EndsWith(".json", StringComparison.OrdinalIgnoreCase))
{
JToken expected = JToken.Parse(File.ReadAllText(existingPath));

byte[] msBytes = new byte[currentMS.Length];
currentMS.Read(msBytes).Should().NotBe(0);

JToken actual = JToken.Parse(System.Text.Encoding.UTF8.GetString(msBytes));

actual.Should().BeEquivalentTo(expected);

return;
}

using FileStream existingFS = new(existingPath, FileMode.Open, FileAccess.Read);

// compare files line by line
Expand Down Expand Up @@ -437,7 +455,7 @@ public GenerationTestsR4(GenerationTestFixture fixture, ITestOutputHelper testOu
[InlineData("OpenApi-Yaml-Inline-Detailed-Candle-Filtered", "TestData/Generated/OpenApi-R4-Inline-Detailed-Candle-Filtered.yaml", "TestData/R4/CapabilityStatement-candle-local.json")]
[Trait("Category", "Generation")]
[Trait("FhirVersion", "R4")]
internal void TestLangR4(string langName, string filePath, string csPath)
internal void TestLangR4(string langName, string filePath, string capabilityJsonPath)
{
// Get the absolute path to the file
string path = Path.IsPathRooted(filePath)
Expand All @@ -449,11 +467,11 @@ internal void TestLangR4(string langName, string filePath, string csPath)
throw new ArgumentException($"Could not find file at path: {path}");
}

csPath = string.IsNullOrEmpty(csPath)
capabilityJsonPath = string.IsNullOrEmpty(capabilityJsonPath)
? string.Empty
: Path.IsPathRooted(csPath)
? csPath
: Path.GetRelativePath(Directory.GetCurrentDirectory(), csPath);
: Path.IsPathRooted(capabilityJsonPath)
? capabilityJsonPath
: Path.GetRelativePath(Directory.GetCurrentDirectory(), capabilityJsonPath);

bool compareLinesDirectly = true;

Expand Down Expand Up @@ -498,18 +516,17 @@ internal void TestLangR4(string langName, string filePath, string csPath)

LangOpenApi exportLang = new();
exportLang.Export(options, _loaded);
compareLinesDirectly = false;
}
break;

case "OpenApi-Json-Inline-None-Candle-Filtered":
{
if (string.IsNullOrEmpty(csPath))
if (string.IsNullOrEmpty(capabilityJsonPath))
{
throw new ArgumentException($"Missing csPath for {langName}");
}

object? csObj = _fixture.Loader?.ParseContents43("application/fhir+json", csPath);
object? csObj = _fixture.Loader?.ParseContents43("application/fhir+json", capabilityJsonPath);

CapabilityStatement? cs = csObj is CapabilityStatement c
? c
Expand All @@ -530,18 +547,17 @@ internal void TestLangR4(string langName, string filePath, string csPath)

LangOpenApi exportLang = new();
exportLang.Export(options, _loaded);
compareLinesDirectly = false;
}
break;

case "OpenApi-Yaml-Inline-None-Candle-Filtered":
{
if (string.IsNullOrEmpty(csPath))
if (string.IsNullOrEmpty(capabilityJsonPath))
{
throw new ArgumentException($"Missing csPath for {langName}");
}

object? csObj = _fixture.Loader?.ParseContents43("application/fhir+json", csPath);
object? csObj = _fixture.Loader?.ParseContents43("application/fhir+json", capabilityJsonPath);

CapabilityStatement? cs = csObj is CapabilityStatement c
? c
Expand All @@ -568,12 +584,12 @@ internal void TestLangR4(string langName, string filePath, string csPath)

case "OpenApi-Yaml-Inline-Names-Candle-Filtered":
{
if (string.IsNullOrEmpty(csPath))
if (string.IsNullOrEmpty(capabilityJsonPath))
{
throw new ArgumentException($"Missing csPath for {langName}");
}

object? csObj = _fixture.Loader?.ParseContents43("application/fhir+json", csPath);
object? csObj = _fixture.Loader?.ParseContents43("application/fhir+json", capabilityJsonPath);

CapabilityStatement? cs = csObj is CapabilityStatement c
? c
Expand All @@ -600,12 +616,12 @@ internal void TestLangR4(string langName, string filePath, string csPath)

case "OpenApi-Yaml-Inline-Detailed-Candle-Filtered":
{
if (string.IsNullOrEmpty(csPath))
if (string.IsNullOrEmpty(capabilityJsonPath))
{
throw new ArgumentException($"Missing csPath for {langName}");
}

object? csObj = _fixture.Loader?.ParseContents43("application/fhir+json", csPath);
object? csObj = _fixture.Loader?.ParseContents43("application/fhir+json", capabilityJsonPath);

CapabilityStatement? cs = csObj is CapabilityStatement c
? c
Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.Health.Fhir.CodeGen.Tests/Microsoft.Health.Fhir.CodeGen.Tests.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

<ItemGroup>
<PackageReference Include="FluentAssertions" Version="6.12.0" />
<PackageReference Include="FluentAssertions.Json" Version="6.1.0" />
<PackageReference Include="Hl7.Fhir.Specification.Data.R5" Version="5.9.0" />
<PackageReference Include="Hl7.Fhir.STU3" Version="5.9.0" />
<PackageReference Include="Hl7.Fhir.R4" Version="5.9.0" />
Expand Down
3 changes: 0 additions & 3 deletions src/Microsoft.Health.Fhir.CodeGen/Loader/PackageLoader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -666,9 +666,6 @@ private bool LoadFromDirectory(ref DefinitionCollection? definitions, string sou

Guid? syncHandle = null;

// put package installation in a named sync object so that multiple spawns do not hammer the package server
// note that we cannot use a mutex because C# async (await) does not maintain the original thread identity
// note that we cannot use a semaphore because .Net does not support named semaphores on all platforms
try
{
string sName = "fcg-" + packageReference.Moniker;
Expand Down
8 changes: 4 additions & 4 deletions src/Microsoft.Health.Fhir.CodeGen/Utils/InterProcessSync.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public InterProcessSync()

long maxTicks = DateTime.Now.Ticks + delay;

while (DateTime.Now.Ticks < delay)
while (DateTime.Now.Ticks < maxTicks)
{
await Task.Delay(100);

Expand Down Expand Up @@ -165,7 +165,7 @@ public async Task<bool> TryWait(Guid handle, int delay = _defaultDelay)

long maxTicks = DateTime.Now.Ticks + delay;

while (DateTime.Now.Ticks < delay)
while (DateTime.Now.Ticks < maxTicks)
{
if (_mutexRequests[handle].State == InterProcessSyncStates.Owned)
{
Expand Down Expand Up @@ -216,11 +216,11 @@ public void ReleaseLock(Guid handle)
/// <summary>
/// Represents the method that will be executed in the mutex management thread.
/// </summary>
private async void MutexManagementThreadFunc()
private void MutexManagementThreadFunc()
{
while (!_disposing)
{
await Task.Delay(100);
System.Threading.Thread.Sleep(100);

foreach (Guid key in _mutexRequests.Keys)
{
Expand Down

0 comments on commit 874cc2e

Please sign in to comment.