Skip to content

Commit ce3bfe7

Browse files
rishabhmalikMSrishabhmalikMS
authored
Fix EOL upgrade warning shown incorrectly for Node 20/24 handler tasks (#5521)
* Fixing EOL node warning display * Add EOL warning assertions to L0 node handler tests * Extract EOL version threshold to NodeVersionHelper.MaxEOLNodeVersion constant
1 parent ee05ba2 commit ce3bfe7

5 files changed

Lines changed: 47 additions & 11 deletions

File tree

‎src/Agent.Worker/NodeVersionStrategies/Node20Strategy.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public NodeRunnerInfo CanHandle(TaskContext context, IExecutionContext execution
4545
NodePath = null,
4646
NodeVersion = NodeVersion.Node20,
4747
Reason = "Upgraded from end-of-life Node version due to EOL policy",
48-
Warning = StringUtil.Loc("NodeEOLUpgradeWarning", taskName)
48+
Warning = context.EffectiveMaxVersion <= NodeVersionHelper.MaxEOLNodeVersion ? StringUtil.Loc("NodeEOLUpgradeWarning", taskName) : null
4949
};
5050
}
5151

‎src/Agent.Worker/NodeVersionStrategies/Node24Strategy.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ public NodeRunnerInfo CanHandle(TaskContext context, IExecutionContext execution
6363
NodePath = null,
6464
NodeVersion = NodeVersion.Node24,
6565
Reason = "Upgraded from end-of-life Node version due to EOL policy",
66-
Warning = StringUtil.Loc("NodeEOLUpgradeWarning", taskName)
66+
Warning = context.EffectiveMaxVersion <= NodeVersionHelper.MaxEOLNodeVersion ? StringUtil.Loc("NodeEOLUpgradeWarning", taskName) : null
6767
};
6868
}
6969

‎src/Agent.Worker/NodeVersionStrategies/NodeRunnerInfo.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@ public enum NodeVersion
2424
/// </summary>
2525
public static class NodeVersionHelper
2626
{
27+
/// <summary>
28+
/// The highest Node version considered end-of-life.
29+
/// Tasks with EffectiveMaxVersion at or below this threshold get an EOL upgrade warning.
30+
/// </summary>
31+
public const int MaxEOLNodeVersion = 16;
32+
2733
/// <summary>
2834
/// Gets the folder name for the specified NodeVersion.
2935
/// </summary>

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

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,8 @@ public static class NodeHandlerTestSpecs
125125
handlerData: typeof(NodeHandlerData),
126126
knobs: new() { ["AGENT_RESTRICT_EOL_NODE_VERSIONS"] = "true" },
127127
legacyExpectedNode: "node",
128-
strategyExpectedNode: "node24"
128+
strategyExpectedNode: "node24",
129+
strategyExpectedWarning: "NodeEOLUpgradeWarning"
129130
),
130131

131132
new TestScenario(
@@ -210,7 +211,8 @@ public static class NodeHandlerTestSpecs
210211
knobs: new() { ["AGENT_RESTRICT_EOL_NODE_VERSIONS"] = "true" },
211212
node24GlibcError: true,
212213
legacyExpectedNode: "node",
213-
strategyExpectedNode: "node20_1"
214+
strategyExpectedNode: "node20_1",
215+
strategyExpectedWarning: "NodeEOLUpgradeWarning"
214216
),
215217

216218
new TestScenario(
@@ -251,7 +253,8 @@ public static class NodeHandlerTestSpecs
251253
handlerData: typeof(Node10HandlerData),
252254
knobs: new() { ["AGENT_RESTRICT_EOL_NODE_VERSIONS"] = "true" },
253255
legacyExpectedNode: "node10",
254-
strategyExpectedNode: "node24"
256+
strategyExpectedNode: "node24",
257+
strategyExpectedWarning: "NodeEOLUpgradeWarning"
255258
),
256259

257260
new TestScenario(
@@ -335,7 +338,8 @@ public static class NodeHandlerTestSpecs
335338
knobs: new() { ["AGENT_RESTRICT_EOL_NODE_VERSIONS"] = "true" },
336339
node24GlibcError: true,
337340
legacyExpectedNode: "node10",
338-
strategyExpectedNode: "node20_1"
341+
strategyExpectedNode: "node20_1",
342+
strategyExpectedWarning: "NodeEOLUpgradeWarning"
339343
),
340344

341345
new TestScenario(
@@ -376,7 +380,8 @@ public static class NodeHandlerTestSpecs
376380
handlerData: typeof(Node16HandlerData),
377381
knobs: new() { ["AGENT_RESTRICT_EOL_NODE_VERSIONS"] = "true" },
378382
legacyExpectedNode: "node16",
379-
strategyExpectedNode: "node24"
383+
strategyExpectedNode: "node24",
384+
strategyExpectedWarning: "NodeEOLUpgradeWarning"
380385
),
381386

382387
new TestScenario(
@@ -386,7 +391,8 @@ public static class NodeHandlerTestSpecs
386391
knobs: new() { ["AGENT_RESTRICT_EOL_NODE_VERSIONS"] = "true" },
387392
node24GlibcError: true,
388393
legacyExpectedNode: "node16",
389-
strategyExpectedNode: "node20_1"
394+
strategyExpectedNode: "node20_1",
395+
strategyExpectedWarning: "NodeEOLUpgradeWarning"
390396
),
391397

392398
new TestScenario(
@@ -427,7 +433,8 @@ public static class NodeHandlerTestSpecs
427433
knobs: new() { ["AGENT_RESTRICT_EOL_NODE_VERSIONS"] = "true" },
428434
node20GlibcError: true,
429435
legacyExpectedNode: "node16",
430-
strategyExpectedNode: "node24"
436+
strategyExpectedNode: "node24",
437+
strategyExpectedWarning: ""
431438
),
432439

433440
new TestScenario(
@@ -495,7 +502,7 @@ public static class NodeHandlerTestSpecs
495502
},
496503
expectedNode: "node24"
497504
),
498-
505+
499506
// ========================================================================================
500507
// GROUP 5: CONTAINER-SPECIFIC EOL SCENARIOS
501508
// ========================================================================================
@@ -638,7 +645,8 @@ public static class NodeHandlerTestSpecs
638645
["AGENT_RESTRICT_EOL_NODE_VERSIONS"] = "true"
639646
},
640647
legacyExpectedNode: "node10",
641-
strategyExpectedNode: "node24"
648+
strategyExpectedNode: "node24",
649+
strategyExpectedWarning: "NodeEOLUpgradeWarning"
642650
),
643651

644652
// ========================================================================================
@@ -855,6 +863,7 @@ public class TestScenario
855863
public string LegacyExpectedNode { get; set; }
856864
public string StrategyExpectedNode { get; set; }
857865
public string StrategyExpectedError { get; set; }
866+
public string StrategyExpectedWarning { get; set; }
858867
public Type ExpectedErrorType { get; set; }
859868

860869
public TestScenario(
@@ -866,6 +875,7 @@ public TestScenario(
866875
string legacyExpectedNode = null,
867876
string strategyExpectedNode = null,
868877
string strategyExpectedError = null,
878+
string strategyExpectedWarning = null,
869879
Type expectedErrorType = null,
870880
bool node20GlibcError = false,
871881
bool node24GlibcError = false,
@@ -882,6 +892,7 @@ public TestScenario(
882892
LegacyExpectedNode = legacyExpectedNode ?? expectedNode;
883893
StrategyExpectedNode = strategyExpectedNode ?? expectedNode;
884894
StrategyExpectedError = strategyExpectedError;
895+
StrategyExpectedWarning = strategyExpectedWarning;
885896
ExpectedErrorType = expectedErrorType;
886897
Node20GlibcError = node20GlibcError;
887898
Node24GlibcError = node24GlibcError;

‎src/Test/L0/NodeHandlerTestBase.cs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ namespace Microsoft.VisualStudio.Services.Agent.Tests
2222
public abstract class NodeHandlerTestBase : IDisposable
2323
{
2424
protected Mock<INodeHandlerHelper> NodeHandlerHelper { get; private set; }
25+
protected List<string> CapturedWarnings { get; private set; } = new List<string>();
2526
private bool disposed = false;
2627

2728
protected NodeHandlerTestBase()
@@ -108,6 +109,19 @@ protected void RunScenarioAndAssert(TestScenario scenario, bool useStrategy)
108109

109110
string expectedLocation = GetExpectedNodeLocation(expectations.ExpectedNode, scenario, thc);
110111
Assert.Equal(expectedLocation, actualLocation);
112+
113+
// Assert warning expectations for strategy-based mode
114+
if (useStrategy && scenario.StrategyExpectedWarning != null)
115+
{
116+
if (string.IsNullOrEmpty(scenario.StrategyExpectedWarning))
117+
{
118+
Assert.DoesNotContain(CapturedWarnings, w => w.Contains("NodeEOLUpgradeWarning"));
119+
}
120+
else
121+
{
122+
Assert.Contains(CapturedWarnings, w => w.Contains(scenario.StrategyExpectedWarning));
123+
}
124+
}
111125
}
112126
catch (Exception ex)
113127
{
@@ -383,6 +397,11 @@ protected Mock<IExecutionContext> CreateTestExecutionContext(TestHostContext tc,
383397
.Setup(x => x.GetHostContext())
384398
.Returns(tc);
385399

400+
CapturedWarnings.Clear();
401+
executionContext
402+
.Setup(x => x.AddIssue(It.Is<Issue>(i => i.Type == IssueType.Warning)))
403+
.Callback<Issue>(issue => CapturedWarnings.Add(issue.Message));
404+
386405
return executionContext;
387406
}
388407

0 commit comments

Comments
 (0)