Skip to content

Commit 0948f72

Browse files
rishabhmalikMSrishabhmalikMS
committed
Code cleaning
1 parent 84e3eb1 commit 0948f72

File tree

2 files changed

+2
-115
lines changed

2 files changed

+2
-115
lines changed

‎src/Test/L0/NodeHandlerL0.TestSpecifications.cs‎

Lines changed: 0 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,6 @@
1010

1111
namespace Microsoft.VisualStudio.Services.Agent.Tests
1212
{
13-
/// <summary>
14-
/// Organization:
15-
/// 1. CUSTOM NODE SCENARIOS (Priority 0 - HIGHEST)
16-
/// 2. NODE6 SCENARIOS (NodeHandlerData - EOL)
17-
/// 3. NODE10 SCENARIOS (Node10HandlerData - EOL)
18-
/// 4. NODE16 SCENARIOS (Node16HandlerData - EOL)
19-
/// 5. NODE20 SCENARIOS (Node20_1HandlerData)
20-
/// 6. NODE24 SCENARIOS (Node24HandlerData)
21-
/// 7. CONTAINER-SPECIFIC SCENARIOS
22-
/// 8. EOL POLICY SCENARIOS
23-
/// 9. EDGE CASES AND ERROR SCENARIOS
24-
/// </summary>
2513
public static class NodeHandlerTestSpecs
2614
{
2715
public static readonly TestScenario[] AllScenarios = new[]
@@ -471,28 +459,6 @@ public static class NodeHandlerTestSpecs
471459
expectSuccess: true,
472460
shouldMatchBetweenModes: true
473461
),
474-
475-
// should this give node24?? as node 24 glibc is compatible in this case (not set so good)
476-
// lets review this test case - review done of both below 2 test,
477-
// below 2 mentioned cases covered in immediate 2 below tests for EOL policy enabled
478-
// 2 cases in this for EOL = true
479-
// 1. node20 glibc error true, node24 glibc error false ( not set) - select node 24 - this test is covered - Node20_GlibcError_EOLPolicy_UpgradesToNode24
480-
// 2. node20 glibc error true, node24 glibc error true - should throw error as both have glibc errors
481-
// new TestScenario( // DONE
482-
// name: "Node20_GlibcError_Node24_GlibcError_EOLPolicy_ThrowsError",
483-
// description: "Node20 and Node24 with glibc error and EOL policy enabled throws error (cannot fallback to Node16), legacy picks Node16",
484-
// handlerData: typeof(Node20_1HandlerData),
485-
// knobs: new() { ["AGENT_ENABLE_EOL_NODE_VERSION_POLICY"] = "true" },
486-
// node20GlibcError: true,
487-
// node24GlibcError: true,
488-
// legacyExpectedNode: "node16", // Legacy falls back to Node16
489-
// legacyExpectSuccess: true,
490-
// expectSuccess: false,
491-
// expectedErrorType: typeof(NotSupportedException),
492-
// unifiedExpectedError: "would fallback to Node16 (EOL) but EOL policy is enabled",
493-
// shouldMatchBetweenModes: false, // Different EOL policy behavior
494-
// ),
495-
// uncomment htis test, giving some syntax error, check and fix
496462

497463
new TestScenario(
498464
name: "Node20_GlibcError_EOLPolicy_UpgradesToNode24",
@@ -675,21 +641,6 @@ public static class NodeHandlerTestSpecs
675641
inContainer: true,
676642
shouldMatchBetweenModes: false
677643
),
678-
679-
new TestScenario( // VERIFY THIS TEST --- containerNeedsNode20Redirect = true so node24 has glibc error on container
680-
// fix this, currently passing incorrectly
681-
name: "Node20_InContainer_WithRedirect_EOLPolicy_UpgradesToNode24",
682-
description: "Node20 in container with redirect and EOL policy upgrades to Node24 (unified) or stays Node20 (legacy)",
683-
handlerData: typeof(Node20_1HandlerData),
684-
knobs: new() { ["AGENT_ENABLE_EOL_NODE_VERSION_POLICY"] = "true" },
685-
// node24GlibcError: true,
686-
legacyExpectedNode: "node20_1", // Legacy stays Node20
687-
legacyExpectSuccess: true,
688-
unifiedExpectedNode: "node24", // Unified upgrades to Node24 due to EOL policy
689-
unifiedExpectSuccess: true,
690-
inContainer: true,
691-
shouldMatchBetweenModes: false
692-
),
693644

694645
new TestScenario(
695646
name: "Node20_AllGlobalKnobsDisabled_UsesHandler",
@@ -936,7 +887,6 @@ public TestScenario(
936887
CustomNodePath = customNodePath;
937888
}
938889

939-
public override string ToString() => Name; // needed?
940890
}
941891

942892
}

‎src/Test/L0/NodeHandlerTestBase.cs‎

Lines changed: 2 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,6 @@
1414

1515
namespace Microsoft.VisualStudio.Services.Agent.Tests
1616
{
17-
/// <summary>
18-
/// Simplified base class for NodeHandler test execution.
19-
/// Sets up test environment, runs tests, and cleans up.
20-
/// </summary>
2117
public abstract class NodeHandlerTestBase : IDisposable
2218
{
2319
protected Mock<INodeHandlerHelper> NodeHandlerHelper { get; private set; }
@@ -47,17 +43,11 @@ protected virtual void Dispose(bool disposing)
4743
}
4844
}
4945

50-
/// <summary>
51-
/// Execute a test scenario and assert the expected behavior.
52-
/// </summary>
5346
protected void RunScenarioAndAssert(TestScenario scenario)
5447
{
5548
RunScenarioAndAssert(scenario, useUnifiedStrategy: false);
5649
}
5750

58-
/// <summary>
59-
/// Execute a test scenario and assert the expected behavior with strategy selection.
60-
/// </summary>
6151
protected void RunScenarioAndAssert(TestScenario scenario, bool useUnifiedStrategy)
6252
{
6353
ResetEnvironment();
@@ -87,7 +77,6 @@ protected void RunScenarioAndAssert(TestScenario scenario, bool useUnifiedStrate
8777

8878
var expectations = GetScenarioExpectations(scenario, useUnifiedStrategy);
8979

90-
// Execute test
9180
if (expectations.ExpectSuccess)
9281
{
9382
string actualLocation = nodeHandler.GetNodeLocation(
@@ -106,7 +95,6 @@ protected void RunScenarioAndAssert(TestScenario scenario, bool useUnifiedStrate
10695
node24ResultsInGlibCError: scenario.Node24GlibcError,
10796
inContainer: scenario.InContainer));
10897

109-
// Verify error message if specified
11098
if (!string.IsNullOrEmpty(expectations.ExpectedError))
11199
{
112100
Assert.Contains(expectations.ExpectedError, exception.Message);
@@ -120,9 +108,6 @@ protected void RunScenarioAndAssert(TestScenario scenario, bool useUnifiedStrate
120108
}
121109
}
122110

123-
/// <summary>
124-
/// Configure NodeHandlerHelper mock based on scenario conditions.
125-
/// </summary>
126111
private void ConfigureNodeHandlerHelper(TestScenario scenario)
127112
{
128113
NodeHandlerHelper.Reset();
@@ -140,11 +125,6 @@ private void ConfigureNodeHandlerHelper(TestScenario scenario)
140125
$"node{IOUtil.ExeExtension}"));
141126
}
142127

143-
/// <summary>
144-
/// Get expected node location based on scenario and expectedNode.
145-
/// For custom node paths, returns the custom path exactly as specified.
146-
/// For standard node paths, constructs the appropriate path based on container vs host.
147-
/// </summary>
148128
private string GetExpectedNodeLocation(string expectedNode, TestScenario scenario, TestHostContext thc)
149129
{
150130
if (!string.IsNullOrWhiteSpace(scenario.CustomNodePath))
@@ -159,9 +139,6 @@ private string GetExpectedNodeLocation(string expectedNode, TestScenario scenari
159139
$"node{IOUtil.ExeExtension}");
160140
}
161141

162-
/// <summary>
163-
/// Get scenario expectations based on whether it's equivalent or divergent and which strategy is being used.
164-
/// </summary>
165142
protected ScenarioExpectations GetScenarioExpectations(TestScenario scenario, bool useUnifiedStrategy)
166143
{
167144
return new ScenarioExpectations
@@ -172,9 +149,6 @@ protected ScenarioExpectations GetScenarioExpectations(TestScenario scenario, bo
172149
};
173150
}
174151

175-
/// <summary>
176-
/// Create handler data instance based on type.
177-
/// </summary>
178152
protected BaseNodeHandlerData CreateHandlerData(Type handlerDataType)
179153
{
180154
if (handlerDataType == typeof(NodeHandlerData))
@@ -191,9 +165,6 @@ protected BaseNodeHandlerData CreateHandlerData(Type handlerDataType)
191165
throw new ArgumentException($"Unknown handler data type: {handlerDataType}");
192166
}
193167

194-
/// <summary>
195-
/// Create test execution context with environment variables.
196-
/// </summary>
197168
protected Mock<IExecutionContext> CreateTestExecutionContext(TestHostContext tc, Dictionary<string, string> knobs)
198169
{
199170
var executionContext = new Mock<IExecutionContext>();
@@ -217,7 +188,6 @@ protected Mock<IExecutionContext> CreateTestExecutionContext(TestHostContext tc,
217188
.Setup(x => x.GetVariableValueOrDefault(It.IsAny<string>()))
218189
.Returns((string variableName) =>
219190
{
220-
// Check variables first, then environment
221191
if (variables.TryGetValue(variableName, out VariableValue value))
222192
{
223193
return value.Value;
@@ -228,14 +198,10 @@ protected Mock<IExecutionContext> CreateTestExecutionContext(TestHostContext tc,
228198
return executionContext;
229199
}
230200

231-
/// <summary>
232-
/// Create test execution context with custom node path support.
233-
/// </summary>
234201
protected Mock<IExecutionContext> CreateTestExecutionContext(TestHostContext tc, TestScenario scenario)
235202
{
236203
var executionContext = CreateTestExecutionContext(tc, scenario.Knobs);
237204

238-
// Setup StepTarget object for custom node path scenarios
239205
if (!string.IsNullOrWhiteSpace(scenario.CustomNodePath))
240206
{
241207
var stepTarget = CreateStepTargetObject(scenario);
@@ -245,7 +211,6 @@ protected Mock<IExecutionContext> CreateTestExecutionContext(TestHostContext tc,
245211
}
246212
else
247213
{
248-
// No custom path - return null StepTarget
249214
executionContext
250215
.Setup(x => x.StepTarget())
251216
.Returns((ExecutionTargetInfo)null);
@@ -254,33 +219,24 @@ protected Mock<IExecutionContext> CreateTestExecutionContext(TestHostContext tc,
254219
return executionContext;
255220
}
256221

257-
/// <summary>
258-
/// Create StepTarget object for custom node scenarios.
259-
/// Uses real HostInfo/ContainerInfo objects since CustomNodePath is not virtual.
260-
/// </summary>
261222
private ExecutionTargetInfo CreateStepTargetObject(TestScenario scenario)
262223
{
263224
if (scenario.InContainer)
264225
{
265-
// Container scenario - create real ContainerInfo with custom node path
266226
return new ContainerInfo()
267227
{
268228
CustomNodePath = scenario.CustomNodePath
269229
};
270230
}
271231
else
272232
{
273-
// Host scenario - create real HostInfo with custom node path
274233
return new HostInfo()
275234
{
276235
CustomNodePath = scenario.CustomNodePath
277236
};
278237
}
279238
}
280239

281-
/// <summary>
282-
/// Get mocked node handler helper with default behavior.
283-
/// </summary>
284240
private Mock<INodeHandlerHelper> GetMockedNodeHandlerHelper()
285241
{
286242
var nodeHandlerHelper = new Mock<INodeHandlerHelper>();
@@ -300,9 +256,6 @@ private Mock<INodeHandlerHelper> GetMockedNodeHandlerHelper()
300256
return nodeHandlerHelper;
301257
}
302258

303-
/// <summary>
304-
/// Reset all node-related environment variables.
305-
/// </summary>
306259
protected void ResetEnvironment()
307260
{
308261
// Core Node.js strategy knobs
@@ -315,38 +268,22 @@ protected void ResetEnvironment()
315268
// EOL and strategy control
316269
Environment.SetEnvironmentVariable("AGENT_ENABLE_EOL_NODE_VERSION_POLICY", null);
317270
Environment.SetEnvironmentVariable("AGENT_USE_UNIFIED_NODE_STRATEGY", null);
318-
Environment.SetEnvironmentVariable("AGENT_DISABLE_NODE6_TASKS", null);
271+
// Environment.SetEnvironmentVariable("AGENT_DISABLE_NODE6_TASKS", null);
319272

320273
// System-specific knobs
321274
Environment.SetEnvironmentVariable("AGENT_USE_NODE20_IN_UNSUPPORTED_SYSTEM", null);
322-
Environment.SetEnvironmentVariable("AGENT_USE_NODE24_IN_UNSUPPORTED_SYSTEM", null);
275+
Environment.SetEnvironmentVariable("AGENT_USE_NODE24_IN_UNSUPPORTED_SYSTEM", null);
323276

324-
// Agent path variables
325-
Environment.SetEnvironmentVariable("VSTS_AGENT_SRC_FOLDER", null);
326-
Environment.SetEnvironmentVariable("AGENT_TOOLSDIRECTORY", null);
327-
328-
// Node warnings
329-
Environment.SetEnvironmentVariable("VSTSAGENT_ENABLE_NODE_WARNINGS", null);
330-
331-
// Force garbage collection to ensure clean state
332-
GC.Collect();
333-
GC.WaitForPendingFinalizers();
334277
}
335278
}
336279

337-
/// <summary>
338-
/// Result of running a test scenario.
339-
/// </summary>
340280
public class TestResult
341281
{
342282
public bool Success { get; set; }
343283
public string NodePath { get; set; }
344284
public Exception Exception { get; set; }
345285
}
346286

347-
/// <summary>
348-
/// Encapsulates expectations for a scenario based on strategy.
349-
/// </summary>
350287
public class ScenarioExpectations
351288
{
352289
public string ExpectedNode { get; set; }

0 commit comments

Comments
 (0)