Adding L0 tests for Legacy NodeHandler logic | Adding 2 Knobs for feature flag & EOL toggle status value #5421

rishabhmalikMS · 2025-12-08T08:53:58Z

Context

This PR introduces comprehensive L0 test specifications for NodeHandler behavior to establish a test-driven development foundation for upcoming EOL (End of Life) Node.js version policy and unified node selection strategy improvements. The test specifications define expected behaviors for both current legacy implementation and future unified strategy implementation.

Also added 2 knobs, one for feature flag usage to switch between legacy and new Node selection process. Another, to read the EOL toggle status value injected by server when user changes task settings from org settings from ADO.

Related Work: This is the first PR in a multi-part effort to modernize Node.js version handling in Azure Pipelines Agent with proper EOL policy enforcement and custom node path support.

Workitems links

Description

Added comprehensive test infrastructure and specifications for NodeHandler with detailed test scenarios covering:

Test Infrastructure:

NodeHandlerTestBase.cs - Clean base class for NodeHandler test execution with environment setup/teardown
NodeHandlerL0.TestSpecifications.cs - Comprehensive test scenarios covering all Node.js versions and edge cases
NodeHandlerL0.AllSpecs.cs - Unified test runner executing all scenarios
NodeHandlerCollections.cs - Test collection configuration for proper isolation

Test Coverage Areas:
2. Node6 Scenarios - Legacy Node 6.x behavior and EOL policy interactions
3. Node10 Scenarios - Node 10.x behavior and EOL policy interactions
4. Node16 Scenarios - Node 16.x behavior and EOL policy interactions
5. Node20 Scenarios - Current default Node 20.x behavior
6. Node24 Scenarios - Latest Node 24.x support
7. Container Scenarios - Node selection in containerized environments
8. EOL Policy Scenarios - End-of-life version handling differences between legacy and unified strategies

Test Structure: Each test scenario includes:

Input configuration (handler type, environment knobs, glibc errors, container mode)
Strategy-specific expectations (legacy vs unified behavior)
Clear success/failure criteria and expected error messages
Please refer TestScenario Field Structure & Usage regarding our test specifications have been defined and used.

Risk Assessment (Low / Medium / High)

Low Risk - This PR contains only test code with no changes to production logic. The tests currently run against existing legacy NodeHandler implementation, with some CustomNode tests intentionally failing to demonstrate implementation gaps for future development.

Unit Tests Added or Updated (Yes / No)

Yes

Additional Testing Performed

List manual or automated tests performed beyond unit tests (e.g., integration, scenario, regression).

Change Behind Feature Flag (Yes / No)

Not applicable - This PR contains only test code with no production changes requiring feature flags.

Tech Design / Approach

Design has been written and reviewed.
Any architectural decisions, trade-offs, and alternatives are captured.

Documentation Changes Required (Yes/No)

Test-First Development Approach:

Comprehensive test specifications define behavior before implementation
Clear separation between legacy and unified strategy expectations
Strategy-agnostic test infrastructure supporting both approaches

Logging Added/Updated (Yes/No)

Appropriate log statements are added with meaningful messages.
Logging does not expose sensitive data.
Log levels are used correctly (e.g., info, warn, error).

Telemetry Added/Updated (Yes/No)

Custom telemetry (e.g., counters, timers, error tracking) is added as needed.
Events are tagged with proper metadata for filtering and analysis.
Telemetry is validated in staging or test environments.

Rollback Scenario and Process (Yes/No)

Rollback plan is documented.

Dependency Impact Assessed and Regression Tested (Yes/No)

All impacted internal modules, APIs, services, and third-party libraries are analyzed.
Results are reviewed and confirmed to not break existing functionality.

…ed node | L0 tests for node handlers

…quence | Preventing leakage of test specific environment settings for knobs between node handler tests (both new and legacy)

…checks from handlers | Adding string tempalates for errors and warning and removing hard-coded erros and warning from code

…implyfying unit test scenarios to avoid using redundant code | Update NodeHandlerTestBase to reflect the simplified test scenario usage for unning tests

… check inside getnodeLocation function) | Added custom node path L0 tests | Updated test base code to integrate custom node tests

rishabhmalikMS · 2025-12-08T09:35:44Z

/azp run

azure-pipelines · 2025-12-08T09:35:58Z

Azure Pipelines successfully started running 1 pipeline(s).

dassayantan24 · 2025-12-08T10:36:13Z

src/Agent.Sdk/Knob/AgentKnobs.cs

+            nameof(EnableEOLNodeVersionPolicy),
+            "When enabled, automatically upgrades tasks using end-of-life Node.js versions (6, 10, 16) to supported versions (Node 20.1 or Node 24). Throws error if no supported versions are available on the agent.",
+            new PipelineFeatureSource("AGENT_ENABLE_EOL_NODE_VERSION_POLICY"),
+            new EnvironmentKnobSource("AGENT_ENABLE_EOL_NODE_VERSION_POLICY"),


Can we please finalize the Knob name here? server side change the Knob name is AGENT_RESTRICT_EOL_NODE_VERSIONS.

Keep you change as is. Will update knob string here in agent code to match: AGENT_RESTRICT_EOL_NODE_VERSIONS

dassayantan24 · 2025-12-08T10:37:11Z

src/Test/L0/NodeHandlerL0.cs


 namespace Microsoft.VisualStudio.Services.Agent.Tests
 {
+    [Collection("Unified NodeHandler Tests")]


Is this intended?

Yes, this is intended.
Reason: For node handler tests, each test requires setting up creatain environment knobs and flags for it. By default, xUnit runs tests in parallel. As a result, the environment settings get leaked from one test to another causing inconsistent test failures. This collection is created to run node handler tests in sequence to prevent this.

sanjuyadav24

Please share a table with all cases mentioned in test cases to validate if any case is not missed and expected behavior in each scenario

sanjuyadav24 · 2025-12-08T10:48:16Z

src/Test/L0/NodeHandlerL0.TestSpecifications.cs

+            ),
+
+            new TestScenario(
+                name: "Node6_DefaultBehavior_EOLPolicyDisabled",


whats the difference between above and this test case?

sanjuyadav24 · 2025-12-08T10:49:14Z

src/Test/L0/NodeHandlerL0.TestSpecifications.cs

+
+            new TestScenario(
+                name: "Node6_WithGlobalUseNode10Knob",
+                description: "Node6 handler with global Node10 knob: legacy uses Node10, unified ignores deprecated knob and uses Node6",


why node10 doesn't overwrite node6 knob?

rishabhmalikMS added 14 commits December 3, 2025 09:46

Initial commit | node handler strategies | orchestrator to get select…

df2b8fc

…ed node | L0 tests for node handlers

Adding collection definition for unning all node handling tests in se…

42d4769

…quence | Preventing leakage of test specific environment settings for knobs between node handler tests (both new and legacy)

Adding selectedNode field in UnifiedNodeContext | Removing duplicate …

a8c7794

…checks from handlers | Adding string tempalates for errors and warning and removing hard-coded erros and warning from code

Adding equivalence and diverging test for legacy and new approach | S…

32becf6

…implyfying unit test scenarios to avoid using redundant code | Update NodeHandlerTestBase to reflect the simplified test scenario usage for unning tests

Node Handler integrtion fix for custom node (moved legacy custom node…

2e9ef63

… check inside getnodeLocation function) | Added custom node path L0 tests | Updated test base code to integrate custom node tests

Merged custom node scnario tests with all other test specs

6b5cc04

Code cleaning

244ebe6

Test scenarios updates

ce9dd01

Minor fixes

b2450ba

Adding CustomNodeHandlerData handler

b1a6dd7

minor fixes

af9b999

L0 unit tests for node handler with all scenarios for node handlers

a95c2ab

minor fix

84e3eb1

Code cleaning

0948f72

rishabhmalikMS added the enhancement label Dec 8, 2025

Merge branch 'master' into users/rishabhmalikMS/EOLnodeL0Tests

ac6b0cf

rishabhmalikMS changed the title ~~Users/rishabhmalik ms/eo lnode l0 tests~~ Adding L0 tests for Legacy NodeHandler logic | Adding 2 Knobs for feature flag & EOL toggle status value Dec 8, 2025

rishabhmalikMS marked this pull request as ready for review December 8, 2025 09:52

rishabhmalikMS requested review from a team as code owners December 8, 2025 09:52

dassayantan24 reviewed Dec 8, 2025

View reviewed changes

rishabhmalikMS mentioned this pull request Dec 8, 2025

Add unified Node.js version selection strategy framework with EOL policy support #5422

Draft

sanjuyadav24 reviewed Dec 8, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Adding L0 tests for Legacy NodeHandler logic | Adding 2 Knobs for feature flag & EOL toggle status value #5421

Adding L0 tests for Legacy NodeHandler logic | Adding 2 Knobs for feature flag & EOL toggle status value #5421

Uh oh!

rishabhmalikMS commented Dec 8, 2025 •

edited

Loading

Uh oh!

rishabhmalikMS commented Dec 8, 2025

Uh oh!

azure-pipelines bot commented Dec 8, 2025

Uh oh!

dassayantan24 Dec 8, 2025

Uh oh!

rishabhmalikMS Dec 8, 2025

Uh oh!

dassayantan24 Dec 8, 2025

Uh oh!

rishabhmalikMS Dec 8, 2025

Uh oh!

sanjuyadav24 left a comment

Uh oh!

sanjuyadav24 Dec 8, 2025

Uh oh!

sanjuyadav24 Dec 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Adding L0 tests for Legacy NodeHandler logic | Adding 2 Knobs for feature flag & EOL toggle status value #5421

Are you sure you want to change the base?

Adding L0 tests for Legacy NodeHandler logic | Adding 2 Knobs for feature flag & EOL toggle status value #5421

Uh oh!

Conversation

rishabhmalikMS commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Description

Risk Assessment (Low / Medium / High)

Unit Tests Added or Updated (Yes / No)

Additional Testing Performed

Change Behind Feature Flag (Yes / No)

Tech Design / Approach

Documentation Changes Required (Yes/No)

Logging Added/Updated (Yes/No)

Telemetry Added/Updated (Yes/No)

Rollback Scenario and Process (Yes/No)

Dependency Impact Assessed and Regression Tested (Yes/No)

Uh oh!

rishabhmalikMS commented Dec 8, 2025

Uh oh!

azure-pipelines bot commented Dec 8, 2025

Uh oh!

dassayantan24 Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

rishabhmalikMS Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

dassayantan24 Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

rishabhmalikMS Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

sanjuyadav24 left a comment

Choose a reason for hiding this comment

Uh oh!

sanjuyadav24 Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

sanjuyadav24 Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rishabhmalikMS commented Dec 8, 2025 •

edited

Loading