Fix the state machine tests on preview.4 and cover the materialized alias nodes

Tests/SampleClients.Tests/WorkshopClientSubscriptionTests.cs

@@ -196,6 +196,14 @@ private static async Task DriveClientAsync(
             // their notification callbacks back to the UI thread and need one to do so
             CreateHandles(form);
 
+            // A sample which throws reports it in a message box, and a modal dialog on the
+            // thread this test drives blocks the message loop - every wait below then runs out
+            // and reports that a control never changed, which says nothing about why. The
+            // watchdog closes the dialog and keeps what it said, so the complaint itself
+            // becomes the failure.
+            using var watchdog = new DialogWatchdog();
+            watchdog.Start();
+
             ConnectServerCtrl connect = WinFormsHarness.GetConnectControl(form);
 
             ISession session = await connect
@@ -217,11 +225,29 @@ private static async Task DriveClientAsync(
 
                 if (client.Arrange != null)
                 {
-                    await client.Arrange(form, ct).ConfigureAwait(true);
+                    try
+                    {
+                        await client.Arrange(form, ct).ConfigureAwait(true);
+                    }
+                    catch (Exception) when (watchdog.Captured.Count > 0)
+                    {
+                        // the sample complained while it was being set up, and whatever the
+                        // arrange step then observed is a consequence of that. Report the
+                        // complaint instead, which is the failure a reader can act on.
+                        Assert.Fail(
+                            $"The {client.Name} client reported an error while it was driven: " +
+                            string.Join(" | ", watchdog.Captured));
+                    }
                 }
 
                 bool arrived = await WaitAsync(() => client.HasNotification(form), ct).ConfigureAwait(true);
 
+                // a complaint the sample made is the better failure, so it is reported first
+                Assert.That(
+                    watchdog.Captured,
+                    Is.Empty,
+                    $"The {client.Name} client reported an error while it was driven.");
+
                 Assert.That(
                     arrived,
                     Is.True,

Tests/SampleNodeManagers.Tests/AliasNamesNodeManagerTests.cs

@@ -122,9 +122,73 @@ await ReportAsync("Browsing the standard TagVariables object", children)
             });
         }
 
+        /// <summary>
+        /// A materialized alias node actually reaches the node it stands for.
+        /// </summary>
+        /// <remarks>
+        /// That the node exists under the category is only half of Part 17 §6.2. What makes it
+        /// an alias rather than an empty placeholder is the <c>AliasFor</c> reference of §8.2,
+        /// and the target it names has to be the node the structural browse path leads to -
+        /// otherwise a §6.2 client which discovers the alias by browsing ends up somewhere
+        /// else than one which called <c>FindAlias</c>.
+        /// </remarks>
+        [Test]
+        [CancelAfter(kTimeout)]
+        public async Task AMaterializedAliasNodeReachesTheNodeItStandsFor(CancellationToken ct)
+        {
+            NodeId aliasNode = await ChildAsync(ObjectIds.TagVariables, "TIC101_PV", ct)
+                .ConfigureAwait(false);
+
+            IReadOnlyList<ReferenceDescription> targets = await SessionOps
+                .BrowseAsync(Session, aliasNode, ct, referenceTypeId: ReferenceTypeIds.AliasFor)
+                .ConfigureAwait(false);
+
+            Assert.That(targets, Is.Not.Empty, "The alias node has an AliasFor reference to its target.");
+
+            NodeId target = ExpandedNodeId.ToNodeId(targets[0].NodeId, Session.NamespaceUris);
+
+            NodeId browsed = await ResolveAsync(ct, Plant, Reactor, TemperatureMeasurement)
+                .ConfigureAwait(false);
+
+            await TestContext.Out
+                .WriteLineAsync($"TIC101_PV --AliasFor--> {target}, browse path -> {browsed}")
+                .ConfigureAwait(false);
+
+            Assert.That(
+                target,
+                Is.EqualTo(browsed),
+                "AliasFor points at the node the structural browse path leads to.");
+        }
+
+        /// <summary>
+        /// The optional Methods the standard category gained are not just nodes: they answer.
+        /// </summary>
+        /// <remarks>
+        /// Materialization creates the Method nodes at the NodeIds the OPC Foundation reserves
+        /// for them, but a node a client cannot call is worth nothing. This calls the one which
+        /// the well known categories ship without, on the standard category rather than on the
+        /// sample's own, which is the case a client with no prior knowledge of this server hits.
+        /// </remarks>
+        [Test]
+        [CancelAfter(kTimeout)]
+        public async Task TheStandardCategoryAnswersTheOptionalVerboseMethod(CancellationToken ct)
+        {
+            AliasNameClient standard = AliasNameClient.OpenStandardTagVariables(Session);
+
+            IReadOnlyList<AliasNameVerboseDataType> found = await standard
+                .FindAliasVerboseAsync("TIC101_PV", null, ct)
+                .ConfigureAwait(false);
+
+            Assert.That(
+                NamesOf(found),
+                Is.EquivalentTo(new[] { "TIC101_PV" }),
+                "FindAliasVerbose answers on the standard category, not only on the sample's own.");
+        }
+
         /// <summary>
         /// A wildcard narrows the search the way Part 17 §6.3.2 defines it.
-        /// </summary>        /// <remarks>
+        /// </summary>
+        /// <remarks>
         /// This is the entire point of the pattern argument: a client which wants the
         /// measured values of the plant asks for them by name shape, rather than reading the
         /// whole inventory and filtering it itself.

Tests/SampleNodeManagers.Tests/StateMachinesNodeManagerTests.cs

@@ -146,6 +146,7 @@ public async Task OperationMachineStartsInItsInitialState(CancellationToken ct)
         {
             string state = await ReadOperationStateNameAsync(ct).ConfigureAwait(false);
             NodeId stateId = await ReadOperationStateIdAsync(ct).ConfigureAwait(false);
+            NodeId offNode = await OperationStateNodeAsync("Off", ct).ConfigureAwait(false);
 
             Assert.Multiple(() => {
                 Assert.That(
@@ -155,7 +156,7 @@ public async Task OperationMachineStartsInItsInitialState(CancellationToken ct)
 
                 Assert.That(
                     stateId,
-                    Is.EqualTo(OperationState(StateMachinesNodeManager.OffState)),
+                    Is.EqualTo(offNode),
                     "CurrentState/Id has to name the state node of the machine's own namespace.");
             });
         }
@@ -190,6 +191,7 @@ public async Task CausesDriveTheDeclaredTransitions(CancellationToken ct)
 
             string running = await ReadOperationStateNameAsync(ct).ConfigureAwait(false);
             NodeId runningId = await ReadOperationStateIdAsync(ct).ConfigureAwait(false);
+            NodeId runningNode = await OperationStateNodeAsync("Running", ct).ConfigureAwait(false);
 
             Assert.Multiple(() => {
                 Assert.That(
@@ -198,7 +200,7 @@ public async Task CausesDriveTheDeclaredTransitions(CancellationToken ct)
                     "Start has to move the machine from Idle to Running.");
                 Assert.That(
                     runningId,
-                    Is.EqualTo(OperationState(StateMachinesNodeManager.RunningState)),
+                    Is.EqualTo(runningNode),
                     "CurrentState/Id has to follow the state.");
             });
 
@@ -471,9 +473,21 @@ public async Task ProgramReturnsToReadyOnItsOwn(CancellationToken ct)
         }
 
         #region Helpers
-        private NodeId OperationState(uint stateId)
+        /// <summary>
+        /// The node of one of the Operation machine's states, found by browsing for it.
+        /// </summary>
+        /// <remarks>
+        /// The state nodes are materialized by the stack, which mints their NodeIds from the
+        /// machine's own identifier and the state's browse name rather than from the numeric
+        /// id the sample declared - that number stays on the node as its state number. So the
+        /// node is browsed for by name instead of being computed, which is also what a client
+        /// comparing CurrentState/Id against a state would have to do.
+        /// </remarks>
+        private async Task<NodeId> OperationStateNodeAsync(string stateName, CancellationToken ct)
         {
-            return new NodeId(stateId, NamespaceIndex(StateMachinesNamespace));
+            NodeId machine = await OperationNodeAsync(ct).ConfigureAwait(false);
+
+            return await ChildAsync(machine, stateName, ct).ConfigureAwait(false);
         }
 
         private Task<NodeId> OperationNodeAsync(CancellationToken ct)

docs/TESTING.md

@@ -317,7 +317,7 @@ What each fixture pins down, in one line:
 | Methods | Argument metadata, the two argument-validation refusals, the ramp, and replacing a running process |
 | NodeManagement | The four Part 4 §5.8 services over a real session: a client creates an object and a variable with attributes, gets a node id from the node manager or asks for one, is refused a duplicate browse name, a taken node id, a non-hierarchical reference and a parent outside the folder the sample opens, deletes what it added and is refused the model, references a node into a second folder and drops the reference again without deleting the node, sees the derived counter follow and a GeneralModelChangeEvent report the folder, and is refused everything on a node manager which never opted in |
 | RoleManagement | What a Part 18 Role is worth: an anonymous session browses the machine and is refused every value, an Observer reads but neither writes nor calls, an Operator does both, an Engineer sees a node an Observer cannot browse, UserRolePermissions reports what the session earns, the role configuration is refused to everyone but a SecurityAdmin on an encrypted channel, and a Role granted at runtime reaches an already open session |
-| AliasNames | What a Part 17 index is worth: the standard TagVariables object answers FindAlias for the whole plant, a wildcard narrows it, a tag name resolves to the node the browse path leads to and back again, the application-defined category tree is browsable below the standard Aliases object and its nested categories serve only their own unit, FindAliasVerbose names the category an entry came from, and the tag list is editable at runtime by a SecurityAdmin on an encrypted channel and by nobody else |
+| AliasNames | What a Part 17 index is worth: the standard TagVariables object answers FindAlias for the whole plant, a wildcard narrows it, a tag name resolves to the node the browse path leads to and back again, the materialized alias nodes carry an AliasFor reference which reaches that same node, the standard category answers the optional FindAliasVerbose it was given, the application-defined category tree is browsable below the standard Aliases object and its nested categories serve only their own unit, FindAliasVerbose names the category an entry came from, and the tag list is editable at runtime by a SecurityAdmin on an encrypted channel and by nobody else |
 | UserAuthentication | UserAccessLevel computed per session, the write refused for anonymous, an unknown user refused a session |
 | PerfTest | The register/offset arithmetic in the node id, nodes synthesized on demand, bounds refused |
 | DataAccess | The segment tree, blocks browsable down to their tags, one block reachable through two paths |