-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WORK: add new Interface to support TreeNodeContext in Parameters.Call… #32
Changes from 2 commits
f3f7c5f
991a747
6a64fe3
3518084
114f4cc
ba5f173
431710b
6bf8d53
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
//----------------------------------------------------------------------- | ||
// <copyright file="TreeWalkerCallbacksV2.cs" company="Microsoft"> | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// </copyright> | ||
// <summary> | ||
// The TreeWalkerCallbacksV2 class implements the ITreeWalkerCallbacksV2 interface. | ||
// </summary> | ||
//----------------------------------------------------------------------- | ||
|
||
namespace Microsoft.Forge.TreeWalker.UnitTests | ||
{ | ||
using System; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
|
||
using Microsoft.Forge.TreeWalker; | ||
using Newtonsoft.Json; | ||
|
||
public class TreeWalkerCallbacksV2 : ITreeWalkerCallbacksV2 | ||
{ | ||
/// <summary> | ||
/// Specified the value after callback. | ||
/// </summary> | ||
public bool ShouldSkipActionsInTreeNode { get; set; } | ||
|
||
public async Task BeforeVisitNode( | ||
Guid sessionId, | ||
string treeNodeKey, | ||
dynamic properties, | ||
dynamic userContext, | ||
string treeName, | ||
Guid rootSessionId, | ||
CancellationToken token) | ||
{ | ||
throw new NotImplementedException("must use V2 API"); | ||
} | ||
|
||
public async Task BeforeVisitNode(TreeNodeContext treeNodeContext) | ||
{ | ||
string serializeProperties = JsonConvert.SerializeObject(treeNodeContext.Properties); | ||
|
||
await Task.Run(() => Console.WriteLine(string.Format( | ||
"OnBeforeVisitNode: SessionId: {0}, TreeNodeKey: {1}, Properties: {2}.", | ||
treeNodeContext.SessionId, | ||
treeNodeContext.TreeNodeKey, | ||
serializeProperties))); | ||
|
||
treeNodeContext.ShouldSkipActionsInTreeNode = this.ShouldSkipActionsInTreeNode; | ||
} | ||
|
||
public Task AfterVisitNode( | ||
Guid sessionId, | ||
string treeNodeKey, | ||
dynamic properties, | ||
dynamic userContext, | ||
string treeName, | ||
Guid rootSessionId, | ||
CancellationToken token) | ||
{ | ||
throw new NotImplementedException("must use V2 API"); | ||
} | ||
|
||
public async Task AfterVisitNode(TreeNodeContext treeNodeContext) | ||
{ | ||
string serializeProperties = JsonConvert.SerializeObject(treeNodeContext.Properties); | ||
|
||
await Task.Run(() => Console.WriteLine(string.Format( | ||
"AfterVisitNode: SessionId: {0}, TreeNodeKey: {1}, Properties: {2}.", | ||
treeNodeContext.SessionId, | ||
treeNodeContext.TreeNodeKey, | ||
serializeProperties))); | ||
|
||
treeNodeContext.ShouldSkipActionsInTreeNode = this.ShouldSkipActionsInTreeNode; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,27 +33,57 @@ public class TreeWalkerUnitTests | |
|
||
private Guid sessionId; | ||
private IForgeDictionary forgeState = new ForgeDictionary(new Dictionary<string, object>(), Guid.Empty, Guid.Empty); | ||
private ITreeWalkerCallbacks callbacks; | ||
private ITreeWalkerCallbacksV2 callbacksV2; // 20201012, switch to V2 callback. | ||
private CancellationToken token; | ||
private TreeWalkerParameters parameters; | ||
private TreeWalkerSession session; | ||
private Dictionary<string, ForgeTree> forgeTrees = new Dictionary<string, ForgeTree>(); | ||
private ConcurrentDictionary<string, Script<object>> scriptCache = new ConcurrentDictionary<string, Script<object>>(); | ||
|
||
|
||
public void TestInitialize(string jsonSchema, string treeName = null) | ||
public void TestInitialize(string jsonSchema, string treeName = null, bool shouldSkipActionsInTreeNode = false) | ||
{ | ||
// Initialize contexts, callbacks, and actions. | ||
this.sessionId = Guid.NewGuid(); | ||
this.forgeState = new ForgeDictionary(new Dictionary<string, object>(), this.sessionId, this.sessionId); | ||
this.callbacks = new TreeWalkerCallbacks(); | ||
TreeWalkerCallbacksV2 treeWalkerCallbacksV2 = new TreeWalkerCallbacksV2(); | ||
treeWalkerCallbacksV2.ShouldSkipActionsInTreeNode = shouldSkipActionsInTreeNode; | ||
this.callbacksV2 = treeWalkerCallbacksV2; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think a single line is cleaner here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. totally agree, changed to this.callbacksV2 = new TreeWalkerCallbacksV2() { ShouldSkipActionsInTreeNode = shouldSkipActionsInTreeNode }; In reply to: 504218674 [](ancestors = 504218674) |
||
this.token = new CancellationTokenSource().Token; | ||
|
||
this.parameters = new TreeWalkerParameters( | ||
this.sessionId, | ||
jsonSchema, | ||
this.forgeState, | ||
this.callbacks, | ||
this.callbacksV2, | ||
this.token) | ||
{ | ||
UserContext = new ForgeUserContext(), | ||
ForgeActionsAssembly = typeof(CollectDiagnosticsAction).Assembly, | ||
InitializeSubroutineTree = this.InitializeSubroutineTree, | ||
TreeName = treeName, | ||
Dependencies = new List<Type>() { typeof(FooEnum) }, | ||
ScriptCache = this.scriptCache | ||
}; | ||
|
||
this.session = new TreeWalkerSession(this.parameters); | ||
} | ||
|
||
public void TestInitialize_WithV2Callbacks_InitializedAsV1(string jsonSchema, string treeName = null, bool shouldSkipActionsInTreeNode = false) | ||
{ | ||
// Initialize contexts, callbacks, and actions. | ||
this.sessionId = Guid.NewGuid(); | ||
this.forgeState = new ForgeDictionary(new Dictionary<string, object>(), this.sessionId, this.sessionId); | ||
TreeWalkerCallbacksV2 treeWalkerCallbacksV2 = new TreeWalkerCallbacksV2(); | ||
treeWalkerCallbacksV2.ShouldSkipActionsInTreeNode = shouldSkipActionsInTreeNode; | ||
ITreeWalkerCallbacks callbacksV1 = treeWalkerCallbacksV2; | ||
this.token = new CancellationTokenSource().Token; | ||
|
||
this.parameters = new TreeWalkerParameters( | ||
this.sessionId, | ||
jsonSchema, | ||
this.forgeState, | ||
callbacksV1, // Passes in ITreeWalkerCallbacks as V1, expects TreeWalkerParameters will recognize the instance is actually V2 and uses it as V2; otherwise throw NotImplementedException | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This comment is too long to be in-line. Please move it to its own line, possibly multi-line. #Closed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
this.token) | ||
{ | ||
UserContext = new ForgeUserContext(), | ||
|
@@ -72,14 +102,14 @@ public void TestInitializeWithForgeTree(ForgeTree forgeTree, string treeName = n | |
// Initialize contexts, callbacks, and actions. | ||
this.sessionId = Guid.NewGuid(); | ||
this.forgeState = new ForgeDictionary(new Dictionary<string, object>(), this.sessionId, this.sessionId); | ||
this.callbacks = new TreeWalkerCallbacks(); | ||
this.callbacksV2 = new TreeWalkerCallbacksV2(); | ||
this.token = new CancellationTokenSource().Token; | ||
|
||
this.parameters = new TreeWalkerParameters( | ||
this.sessionId, | ||
forgeTree, | ||
this.forgeState, | ||
this.callbacks, | ||
this.callbacksV2, | ||
this.token) | ||
{ | ||
UserContext = new ForgeUserContext(), | ||
|
@@ -202,6 +232,32 @@ public void TestTreeWalkerSession_WalkTree_ActionThrowsException_TimeoutOnAction | |
Assert.AreEqual("TimeoutOnAction", actualStatus, "Expected WalkTree to timeout on action because the Action threw exceptions with no Continuation flags set."); | ||
} | ||
|
||
#region shouldSkipActionsInTreeNode | ||
|
||
[TestMethod] | ||
public void TestTreeWalkerSession_WalkTree_ActionThrowsException_TimeoutOnAction_Skip() | ||
{ | ||
// Initialize TreeWalkerSession with a schema containing Action that throws exception. | ||
this.TestInitialize(jsonSchema: ForgeSchemaHelper.ActionException_Fail, shouldSkipActionsInTreeNode: true); | ||
|
||
// Test - WalkTree and expect the Status to be RanToCompletion, because it will skip any action including the one throwing TimeoutOnAction. | ||
string actualStatus = this.session.WalkTree("Root").GetAwaiter().GetResult(); | ||
Assert.AreEqual("RanToCompletion", actualStatus, "Expected WalkTree to run to completion."); | ||
} | ||
|
||
[TestMethod] | ||
public void TestTreeWalkerSession_WalkTree_ActionThrowsException_TimeoutOnAction_Skip_V1_AutoCast_To_V2() | ||
{ | ||
// Initialize TreeWalkerSession with a schema containing Action that throws exception. | ||
this.TestInitialize_WithV2Callbacks_InitializedAsV1(jsonSchema: ForgeSchemaHelper.ActionException_Fail, shouldSkipActionsInTreeNode: true); | ||
|
||
// Test - WalkTree and expect the Status to be RanToCompletion, because it will skip any action including the one throwing TimeoutOnAction. | ||
string actualStatus = this.session.WalkTree("Root").GetAwaiter().GetResult(); | ||
Assert.AreEqual("RanToCompletion", actualStatus, "Expected WalkTree to run to completion."); | ||
} | ||
|
||
#endregion shouldSkipActionsInTreeNode | ||
|
||
[TestMethod] | ||
public void TestTreeWalkerSession_WalkTree_ActionThrowsException_ContinuationOnRetryExhaustion() | ||
{ | ||
|
@@ -1125,7 +1181,7 @@ private TreeWalkerSession InitializeSubroutineTree(SubroutineInput subroutineInp | |
subroutineSessionId, | ||
forgeTrees[subroutineInput.TreeName], | ||
new ForgeDictionary(new Dictionary<string, object>(), parentParameters.RootSessionId, subroutineSessionId), | ||
this.callbacks, | ||
this.callbacksV2, | ||
parentParameters.Token) | ||
{ | ||
UserContext = parentParameters.UserContext, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this comment. I don't think timestamped comments when code was changed are helpful. #Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed.
In reply to: 505915856 [](ancestors = 505915856)