Fix case-insensitive Service Fabric durability parsing - #30015
Open
Aditya Pujara (a0x1ab) with Copilot wants to merge 5 commits into
Open
Fix case-insensitive Service Fabric durability parsing#30015Aditya Pujara (a0x1ab) with Copilot wants to merge 5 commits into
Aditya Pujara (a0x1ab) with Copilot wants to merge 5 commits into
Conversation
Contributor
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix Update-AzServiceFabricDurability for lowercase durability values
Fix case-insensitive Service Fabric durability parsing
Aug 14, 2026
Comment on lines
+29
to
+34
| private sealed class TestServiceFabricClusterCmdlet : ServiceFabricClusterCmdlet | ||
| { | ||
| public DurabilityLevel GetNodeTypeDurabilityLevel(string durabilityLevel) | ||
| { | ||
| return GetDurabilityLevel(durabilityLevel); | ||
| } |
Live test results — TestFx
|
![azure-client-tools-agent[bot]](https://avatars.githubusercontent.com/in/4205534?s=60&v=4){kind=small}
There was a problem hiding this comment.
Live-test review — build failure
The TestFx (Record) live test for the changed test file failed during the build step, before any test could run.
Test project: src/ServiceFabric/ServiceFabric.Test
Changed test file: src/ServiceFabric/ServiceFabric.Test/ScenarioTests/TestServiceFabric.cs
Filter: FullyQualifiedName~TestServiceFabric
Build errors (6 total):
src/ServiceFabric/ServiceFabric.Test/ScenarioTests/TestServiceFabric.cs(31,20): error CS0246: The type or namespace name 'DurabilityLevel' could not be found (are you missing a using directive or an assembly reference?)
src/ServiceFabric/ServiceFabric.Test/ScenarioTests/TestServiceFabric.cs(36,20): error CS0246: The type or namespace name 'DurabilityLevel' could not be found (are you missing a using directive or an assembly reference?)
src/ServiceFabric/ServiceFabric.Test/ScenarioTests/TestServiceFabric.cs(89,31): error CS0103: The name 'DurabilityLevel' does not exist in the current context
src/ServiceFabric/ServiceFabric.Test/ScenarioTests/TestServiceFabric.cs(90,31): error CS0103: The name 'DurabilityLevel' does not exist in the current context
src/ServiceFabric/ServiceFabric.Test/ScenarioTests/TestServiceFabric.cs(91,29): error CS0103: The name 'DurabilityLevel' does not exist in the current context
src/ServiceFabric/ServiceFabric.Test/ScenarioTests/TestServiceFabric.cs(92,86): error CS0246: The type or namespace name 'DurabilityLevel' could not be found (are you missing a using directive or an assembly reference?)
The test file references a DurabilityLevel type/enum that either doesn't exist in the referenced SDK/namespace or is missing a using directive. Since this PR's purpose is "Fix case-insensitive Service Fabric durability parsing," please:
- Confirm the correct fully-qualified type/enum for durability level values in the Service Fabric management SDK used by this module (it may need a
using Microsoft.Azure.Management.ServiceFabric.Models;or similar, or the enum name may have changed/been renamed). - Update
TestServiceFabric.cs(and any production code undersrc/ServiceFabric/ServiceFabric.PowerShell.Cmdletsit depends on) so the project builds cleanly. - Push the fix so CI/live-test can re-run.
Live-test run: https://github.com/Azure/issue-sentinel/actions/runs/31827071365
Regular CI checks on this PR are currently green, but the live test build failure must be resolved before this can be merged.
Posted by agent-assist (autonomous bug-fix pipeline).
azure-client-tools-agent
Bot
added
the
azure-client-tools-agent
|
Started a Copilot task using Posted by agent-assist (autonomous bug-fix pipeline). |
…Fabric.cs Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- src/ServiceFabric/ServiceFabric/Properties/Resources.Designer.cs: Generated file
Suppressed comments (2)
src/ServiceFabric/ServiceFabric/Commands/ServiceFabricClusterCmdlet.cs:134
Enum.TryParsewill returntruefor numeric strings (e.g., "1" or "999"), producing enum values that are not actually defined (or bypassing the intended "valid values" check). That means malformed stored durability properties may still slip through instead of throwing the new actionable error. Consider additionally validating that the parsed value is a defined enum name (and trimming input) before returning it.
protected DurabilityLevel GetDurabilityLevel(string durabilityLevel)
{
DurabilityLevel parsedDurabilityLevel;
if (Enum.TryParse(durabilityLevel, true, out parsedDurabilityLevel))
{
src/ServiceFabric/ServiceFabric.Test/ScenarioTests/TestServiceFabric.cs:110
- The negative-path test only exercises a non-numeric invalid value ("invalid"). Since
Enum.TryParsealso accepts numeric strings, add coverage for numeric/undefined inputs (e.g., "1" and "999") to ensure malformed stored durability values reliably trigger the actionable error instead of being interpreted as enum integers.
[Fact]
public void GetDurabilityLevelReportsInvalidValue()
{
var cmdlet = new TestServiceFabricClusterCmdlet();
Live test results — TestFx
|
There was a problem hiding this comment.
Automated Review — Live Test Failed
The dispatched live test (TestFx Record, PowerShell live-test workflow) for
this PR's changed test file failed on head SHA 9630dadcde8e8d522bc0a795efc7af0a7525dead.
Test project: src/ServiceFabric/ServiceFabric.Test
Filter: FullyQualifiedName~TestServiceFabric
Changed test file: src/ServiceFabric/ServiceFabric.Test/ScenarioTests/TestServiceFabric.cs
Result: ❌ FAIL (exit code 1)
Run: https://github.com/Azure/issue-sentinel/actions/runs/31831639663
Relevant error from the test output:
/home/runner/.nuget/packages/microsoft.identity.client.nativeinterop/0.20.4/lib/netstandard2.0/libmsalruntime: cannot open shared object file: No such file or directory
at Microsoft.Azure.Commands.ServiceFabric.Commands.ServiceFabricClusterCmdlet.GetCurrentCluster() in src/ServiceFabric/ServiceFabric/Commands/ServiceFabricClusterCmdlet.cs:line 88
at Microsoft.Azure.Commands.ServiceFabric.Commands.GetAzureRmServiceFabricCluster.ExecuteCmdlet() in src/ServiceFabric/ServiceFabric/Commands/GetAzureRmServiceFabricClusterResouce.cs:line 54
Script Stack Trace: at WaitForClusterReadyState, .../ScenarioTests/Common.ps1: line 181
at WaitForClusterReadyStateIfRecord, .../ScenarioTests/Common.ps1: line 170
at Test-AddAzureRmServiceFabricNodeType, .../ScenarioTests/TestServiceFabric.ps1: line 120
The failing scenario is Test-AddAzureRmServiceFabricNodeType, which calls
GetAzureRmServiceFabricCluster → ServiceFabricClusterCmdlet.GetCurrentCluster().
The exception is cannot open shared object file for
libmsalruntime, which surfaces from MSAL's native interop layer during
authentication in the test cmdlet path — this may point at an MSAL/auth
dependency resolution issue triggered by this PR's change, or at a native
library not being restored/copied to the test output directory. Please
investigate why libmsalruntime isn't being found from the test bin output at
runtime for this scenario, verify the fix for AzServiceFabricDurability case
sensitivity doesn't affect MSAL/auth dependency loading, and push a fix (or
confirm this is a pre-existing environment issue and adjust/re-run
accordingly). CI checks on the PR itself are all green; only this live test
step is failing.
Posted by agent-assist (autonomous bug-fix pipeline).
|
Started a Copilot task using Posted by agent-assist (autonomous bug-fix pipeline). |
…check and numeric test coverage Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
Comment on lines
+132
to
+137
| DurabilityLevel parsedDurabilityLevel; | ||
| if (Enum.TryParse(durabilityLevel?.Trim(), true, out parsedDurabilityLevel) && | ||
| Enum.IsDefined(typeof(DurabilityLevel), parsedDurabilityLevel)) | ||
| { | ||
| return parsedDurabilityLevel; | ||
| } |
Live test results — TestFx
|
There was a problem hiding this comment.
Automated CI + Live-Test Review
GitHub CI checks: 2/2 passed ✅
Live test (TestFx Record, changed test files only): ❌ FAIL (exit 1)
- Test project:
src/ServiceFabric/ServiceFabric.Test - Filter:
FullyQualifiedName~TestServiceFabric - Changed test file:
src/ServiceFabric/ServiceFabric.Test/ScenarioTests/TestServiceFabric.cs - Workflow run: https://github.com/Azure/issue-sentinel/actions/runs/31835478213
The dotnet test run fails during test setup/authentication with:
Unable to load shared library 'msalruntime' or one of its dependencies ...
System.DllNotFoundException: msalruntime.so: cannot open shared object file: No such file or directory
This same failure occurred on the previous run as well
(https://github.com/Azure/issue-sentinel/actions/runs/31831639663), so it is
reproducible for this branch/test project, not a one-off flake.
Please investigate whether:
- The
TestServiceFabricscenario test (or itsServiceFabric.Testproject
setup/fixtures) needs an update to work with the current MSAL/native
dependency setup used by the test harness, or - The PR's change to case-insensitive durability parsing needs an
accompanying update to the test recordings/fixtures so the affected test
can run and pass in this environment.
Please push a fix so the TestServiceFabric tests in
src/ServiceFabric/ServiceFabric.Test pass under dotnet test.
Posted by agent-assist (autonomous bug-fix pipeline).
|
Started a Copilot task using Posted by agent-assist (autonomous bug-fix pipeline). |
… numeric string rejection Co-authored-by: a0x1ab <59631311+a0x1ab@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- src/ServiceFabric/ServiceFabric/Properties/Resources.Designer.cs: Generated file
Suppressed comments (2)
src/ServiceFabric/ServiceFabric/ChangeLog.md:21
- Changelog entry wording is awkward/redundant: “lowercase casing”. Consider simplifying to “stored in lowercase” to improve readability for users.
* Fixed `Update-AzServiceFabricDurability` to accept durability values stored with lowercase casing.
src/ServiceFabric/ServiceFabric.Test/ScenarioTests/TestServiceFabric.cs:22
- Unused
using Microsoft.Azure.Commands.ServiceFabric.Models;directive; this will produce CS8019 (and may fail the build if warnings are treated as errors). Remove it since nothing in this file referencesMicrosoft.Azure.Commands.ServiceFabric.Models.
using Microsoft.Azure.Commands.ServiceFabric.Commands;
using Microsoft.Azure.Commands.ServiceFabric.Common;
using Microsoft.Azure.Commands.ServiceFabric.Models;
using Microsoft.WindowsAzure.Commands.ScenarioTest;
using Xunit;
Live test results — TestFx
|
There was a problem hiding this comment.
CI / Live Test Summary — PR #30015
Live test (TestFx Record, src/ServiceFabric/ServiceFabric.Test): ❌ FAIL (exit code 1)
- Filter:
FullyQualifiedName~DurabilityLevelTests|FullyQualifiedName~TestServiceFabric - Changed test files run:
DurabilityLevelTests.cs,TestServiceFabric.cs - Workflow run: https://github.com/Azure/issue-sentinel/actions/runs/31838719976
- A detailed results comment (with the last 80 lines of
dotnet testoutput) was already posted to this PR by the live-test workflow.
Required GitHub checks: ✅ all passed (0 failed, 0 pending)
The live test run for the Service Fabric durability tests is failing on this PR's head commit
(2adad843dab33102797aa83abf3b26b089b3e87c). Please inspect the failing DurabilityLevelTests/
TestServiceFabric assertions in src/ServiceFabric/ServiceFabric.Test, using the failure output in
the live-test workflow's comment on this PR, and adjust the fix (or the recorded test session/mock
data) so the case-insensitive durability parsing change makes these tests pass. Push a new commit
once fixed so the live test can be re-run on the updated head.
Posted by agent-assist (autonomous bug-fix pipeline).
|
Started a Copilot task using Posted by agent-assist (autonomous bug-fix pipeline). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Update-AzServiceFabricDurabilityfailed beforeShouldProcesswhen existing node type or VMSS extension durability values used lowercase casing (for example,silver).Shared behavior
Coverage