Set ContinuationPoint (when browse is finished) to null instead of empty array, align with existing 1.5.xxx behavior#4057
Merged
Conversation
Flip the 'managedPool' parameter default from false to true so Windows/Linux jobs route to the netstandard Managed DevOps Pool via ImageOverride demands. Operators can still override to Microsoft-hosted agents per queue via the pipeline UI.
Verify default registration keeps classic and async factory paths separate and the Reference Server helper flags remain reusable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add the current ISA95 JobControl NodeSet2 input as a focused source-generator regression without changing generator or codec behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the incomplete resource with the unmodified official ISA95 JobControl 2.0.0 NodeSet and assert its model, node coverage, and local references. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Verify cancellation preserves the caller token and stops the connect attempt started by ManagedSession.CreateAsync. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the caller-visible cancellation and primary disposal path covered while excluding only the defensive secondary disposal-failure classifier. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…upstream-parity-session-cleanup # Conflicts: # Libraries/Opc.Ua.Client/Session/ManagedSession.cs # Tests/Opc.Ua.Server.Tests/QuickstartsServerUtilitiesTests.cs
…enable-managed-pool
…upstream-parity-session-cleanup # Conflicts: # tests/Opc.Ua.SourceGeneration.Core.Tests/Resources/Isa95JobControl.NodeSet2.xml
…ation#3998) ## Summary - Adds the official ISA95 JobControl 2.0.0 NodeSet2 as a complete source-generator regression fixture. - Extends the lightweight generated-stack test stub with `NamespaceMetadataState`, required by the complete model. - Adds a deterministic ManagedSession cancellation regression proving that a started connect attempt is stopped. The ManagedSession cleanup implementation and Quickstarts utility coverage now come from `master`; this PR retains the additional deterministic cleanup regression. ## ISA95 fixture provenance and integrity The fixture is byte-for-byte the official file: - Source: `OPCFoundation/UA-Nodeset@e0b5d80cfff698f0276393e28df138a7fbc4b541` - Path: `ISA95-JOBCONTROL/opc.ua.isa95-jobcontrol.nodeset2.xml` - Git blob: `82f008cd126ade54e5f2c737bcc116621a8c3087` - SHA-256: `52327e736beb604c7253a5a5393d1ad525b9ecf35b71fbacb4f429cacb334064` The fixture test retains the OPC Foundation MIT header and verifies the 2.0.0 model/date, 258 nodes (including 134 variables), representative late variables, and that every `ns=1` reference target resolves locally. ## Validation - ManagedSession tests: 17 passing on net10.0; 15 passing and 2 framework-gated skips on net48. - Quickstarts server utility tests: 3 passing on net10.0 and net48. - ISA95 generator and fixture integrity regression: 5 passing on net10.0.
Replace the all-or-nothing `managedPool` boolean with a `poolMode` enum (mixed | managed | hosted) and route stages across two pool profiles so the netstandard Managed DevOps Pool and the Microsoft-hosted agents share the workload of a single run. - mixed (default): each stage uses its assigned profile, so both pools run in parallel. The two always-concurrent PR stages are split across pools: build -> hosted (bigger temp disk for multi-TFM bin trees), testfastpr -> managed (fast, image-pinned gate). Full-run stages are balanced ~5/5. - managed / hosted: global overrides that collapse both profiles onto a single pool (all-managed or all-hosted) for emergencies. The pool switch lives only in the variables block (mgd*/host* profiles), so stages carry no per-stage conditionals. macOS is unchanged (always hosted; the managed pool has no macOS images).
# Description Flip the `managedPool` parameter default in `azure-pipelines.yml` from `false` to `true` so Windows/Linux jobs route to the `netstandard` Managed DevOps Pool via `ImageOverride` demands. macOS jobs continue to run on Microsoft-hosted agents because Managed DevOps Pools does not provide macOS images. Operators can still uncheck the parameter at queue time to fall back to Microsoft-hosted agents. ## Related issues Follow-up to OPCFoundation#3881. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. _(N/A — pipeline default only.)_ - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. _(N/A — no source change.)_ - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received.
# Description Fail reverse-connect listener startup immediately instead of logging bind failures and surfacing a later connection timeout. - Validate and stage configured and programmatic listener endpoints before replacing active configuration. - Open all listeners atomically and report endpoint-specific failures with `BadTcpEndpointUrlInvalid`, the transport-specific status, or `BadNoCommunication`. - Roll back listeners opened by a failed attempt without reacquiring the manager lock, including forced listener disposal when normal close fails. - Preserve meaningful `ServiceResultException` status codes through both `StartService` overloads. - Dispose replaced configuration watchers and reject callbacks from stale watcher generations. - Keep the intended shared-listener behavior: one long-lived `ReverseConnectManager` can accept connections from multiple Servers on the same Client URL. - Expand `Docs/ReverseConnect.md` with listener lifetime, shared-manager usage, diagnostics, and specification references. ## Validation - `ReverseConnectManagerTests`: net10.0 and net48. - `ReverseConnectHostTests`: net10.0 and net48. - Ordered reverse-connect listener reuse integration tests: net10.0. ## Related Issues - Relates to OPCFoundation#3985. ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Standardizes BrowseResult.ContinuationPoint semantics so “no continuation point” is represented as null/default instead of an “empty” value, and aligns tests with the updated contract.
Changes:
- Initialize
BrowseResult.ContinuationPointtodefaultinMasterNodeManagerfor both good and error results. - Update test assertions to use
ContinuationPoint.IsNullinstead ofIsEmpty. - Add deterministic test coverage for null continuation points in completed/error browse flows.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs |
Explicitly sets ContinuationPoint = default when creating BrowseResult instances. |
tests/Opc.Ua.Server.Tests/NodeManager/MasterNodeManagerDeterministicTests.cs |
Adds/updates assertions and adds a new test validating null continuation points. |
tests/Opc.Ua.InformationModel.Tests/BrowseTests.cs |
Updates continuation point assertion to IsNull. |
tests/Opc.Ua.InformationModel.Tests/BrowseContinuationPointTests.cs |
Updates multiple assertions to IsNull for “no continuation point” cases. |
marcschier
approved these changes
Jul 20, 2026
koepalex
changed the title
null instead of empty array, align with existing 1.5.xxx behavior
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (66.66%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #4057 +/- ##
==========================================
- Coverage 73.86% 73.56% -0.31%
==========================================
Files 1345 1345
Lines 180038 180093 +55
Branches 31678 31683 +5
==========================================
- Hits 132993 132484 -509
- Misses 36290 36900 +610
+ Partials 10755 10709 -46
🚀 New features to boost your workflow:
|
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.
This pull request standardizes the handling of the
ContinuationPointproperty inBrowseResultobjects, ensuring it is always explicitly set tonull(or its equivalent) when no continuation point is present. This change improves clarity and consistency in both the implementation and the tests, replacing checks for an "empty" continuation point with checks for a "null" continuation point.Codebase consistency and correctness:
BrowseResultobjects now explicitly setContinuationPoint = defaultwhen no continuation point is to be returned, ensuring consistent initialization. (src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs) [1] [2] [3]Test updates for new semantics:
All test assertions now use
ContinuationPoint.IsNullinstead ofIsEmptyto check for the absence of a continuation point, reflecting the new contract. (tests/Opc.Ua.InformationModel.Tests/BrowseContinuationPointTests.cs,tests/Opc.Ua.Server.Tests/NodeManager/MasterNodeManagerDeterministicTests.cs) [1] [2] [3] [4] [5]Additional test cases have been added to verify that completed browse operations and error scenarios always return a null continuation point. (
tests/Opc.Ua.Server.Tests/NodeManager/MasterNodeManagerDeterministicTests.cs) [1] [2] [3]# DescriptionDescribe the changes here to communicate to the maintainers why they should accept this pull request. By default - this will become the Commit message after merging and thus define history.
Related Issues
Reference all GitHub issues this PR addresses. If there is no issue yet, open one and link it here.
If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR)).
Checklist
Put an
xin the boxes that apply. You can complete these step by step after opening the PR.