[duplicate-code] Repeated Bool-Parsing Pattern in MSTestSettings.RunSettingsXml.cs

#9871Opengithub-actions[bot] 创建于 2026-07-12

type/tech-debttype/automation

## Summary The `ToSettings` method in `src/Adapter/MSTestAdapter.PlatformServices/MSTestSettings.RunSettingsXml.cs` contains **8 identical structural duplications** of a bool-parsing pattern. This is the same problem that `ParseTimeoutSetting` already solved for integer timeout settings — the bool equivalent has not been extracted into a helper. ## Duplication Details ### Pattern: Read XML element → try-parse bool → set property or log warning - **Severity**: Medium - **Occurrences**: 8 instances - **Location**: `src/Adapter/MSTestAdapter.PlatformServices/MSTestSettings.RunSettingsXml.cs` (lines 104–222) Each case follows this identical structure (~10 lines each = ~80 lines of structural duplication): ```csharp // Repeated 8 times with only variable names and property names changing: string <varName> = reader.ReadInnerXml(); if (bool.TryParse(<varName>, out result)) { settings.<Property> = result; } else { logger?.SendMessage(MessageLevel.Warning, string.Format(CultureInfo.CurrentCulture, Resource.InvalidValue, <varName>, "<XmlElementName>")); } break; ``` **Affected switch cases** (all in `ToSettings`, lines 100–253): | Line | Case | Property | |------|------|----------| | 104 | `CAPTURETRACEOUTPUT` | `settings.CaptureDebugTraces` | | 116 | `MAPINCONCLUSIVETOFAILED` | `settings.MapInconclusiveToFailed` | | 128 | `MAPNOTRUNNABLETOFAILED` | `settings.MapNotRunnableToFailed` | | 140 | `TREATDISCOVERYWARNINGSASERRORS` | `settings.TreatDiscoveryWarningsAsErrors` | | 162 | `CONSIDEREMPTYDATASOURCEASINCONCLUSIVE` | `settings.ConsiderEmptyDataSourceAsInconclusive` | | 189 | `COOPERATIVECANCELLATIONTIMEOUT` | `settings.CooperativeCancellationTimeout` | | 201 | `ORDERTESTSBYNAMEINCLASS` | `settings.OrderTestsByNameInClass` | | 213 | `RANDOMIZETESTORDER` | `settings.RandomizeTestOrder` | Note: `CONSIDEREMPTYDATASOURCEASINCONCLUSIVE` (line 162) uses a slightly different local variable name but follows the same pattern. The **existing** `ParseTimeoutSetting` helper (lines 260–270) already shows the intended pattern — a private helper that takes the raw value, the setting name, a logger, and a setter action: ```csharp private static void ParseTimeoutSetting(string rawValue, string settingName, IAdapterMessageLogger? logger, Action<int> setSetting) { if (int.TryParse(rawValue, out int result) && result > 0) { setSetting(result); } else { logger?.SendMessage(MessageLevel.Warning, string.Format(CultureInfo.CurrentCulture, Resource.InvalidTimeoutValue, rawValue, settingName)); } } ``` ## Impact Analysis - **Maintainability**: Adding a new bool runsettings option requires copying ~10 lines instead of calling a one-liner helper. Any change to the warning message format must be applied in 8 places. - **Bug Risk**: The `CONSIDEREMPTYDATASOURCEASINCONCLUSIVE` case (line 162) already diverged slightly (uses `out bool parsedConsiderEmptyDataSourceAsInconclusive` instead of `out result`), showing how inconsistencies creep in under copy-paste programming. - **Code Bloat**: ~70 lines of switch body could be reduced to ~8 one-liner calls. ## Refactoring Recommendations ### 1. Add a `ParseBoolSetting` helper (mirrors `ParseTimeoutSetting`) ```csharp private static void ParseBoolSetting(string rawValue, string settingName, IAdapterMessageLogger? logger, Action<bool> setSetting) { if (bool.TryParse(rawValue, out bool result)) { setSetting(result); } else { logger?.SendMessage(MessageLevel.Warning, string.Format(CultureInfo.CurrentCulture, Resource.InvalidValue, rawValue, settingName)); } } ``` ### 2. Replace each duplicated block with a call ```csharp case "CAPTURETRACEOUTPUT": ParseBoolSetting(reader.ReadInnerXml(), "CaptureTraceOutput", logger, v => settings.CaptureDebugTraces = v); break; case "MAPINCONCLUSIVETOFAILED": ParseBoolSetting(reader.ReadInnerXml(), "MapInconclusiveToFailed", logger, v => settings.MapInconclusiveToFailed = v); break; // ... and so on for all 8 cases ``` - **Estimated effort**: ~30 minutes - **Benefits**: Eliminates ~70 lines; any future bool setting is a single-line call; no risk of per-case divergence ## Implementation Checklist - [ ] Add `ParseBoolSetting` private static method to `MSTestSettings` partial class (in `MSTestSettings.RunSettingsXml.cs`) - [ ] Replace all 8 duplicated bool-parsing blocks with `ParseBoolSetting(...)` calls - [ ] Verify the `CONSIDEREMPTYDATASOURCEASINCONCLUSIVE` case is handled consistently - [ ] Run existing unit/integration tests to confirm no regression ## Analysis Metadata - **Analyzed Files**: `src/Adapter/MSTestAdapter.PlatformServices/MSTestSettings.RunSettingsXml.cs` - **Detection Method**: Semantic code analysis — structural pattern matching - **Commit**: fa9bbfab33d3f3f9ef8c12a4f9e94339f1c0f1c6 - **Analysis Date**: 2026-07-12 > 🤖 **Automated content by GitHub Copilot.** Generated by the [Duplicate Code Detector](https://github.com/microsoft/testfx/actions/runs/29180838481/agentic_workflow) workflow. · 70.9 AIC · ⌖ 8.45 AIC · ⊞ 8K · [◷]( · [◷](https://github.com/search?q=repo%3Amicrosoft%2Ftestfx+is%3Aissue+%22gh-aw-workflow-call-id%3A+microsoft%2Ftestfx%2Fduplicate-code-detector%22&type=issues)) > <details> <summary><sub>Add this agentic workflow to your repo</sub></summary> To install this agentic workflow, run ``` gh aw add githubnext/agentics/workflows/duplicate-code-detector.md@main ``` </details> > - [x] expires  on Jul 14, 2026, 5:17 AM UTC

0 条评论