Skip to content

Commit

Permalink
Fix crash on Node2code operation in case of conflicts with non-unique…
Browse files Browse the repository at this point in the history 
… namespaces (#9255)

* fix crash on node2code in case of conflicts with non-unique namespaces

* test cleanup and add new test
  • Loading branch information
@aparajit-pratap
aparajit-pratap authored Nov 21, 2018
1 parent aae0efb commit 896e0b2
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 43 deletions.
4 changes: 4 additions & 0 deletions src/Engine/ProtoCore/Namespace/SymbolTable.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,10 @@ public static Dictionary<Symbol, string> GetShortestUniqueNames(
break;
}
Debug.Assert(!shortNamespaces.ContainsKey(symbol));
if(string.IsNullOrEmpty(shortName))
{
shortName = symbol.FullName;
}
shortNamespaces.Add(symbol, shortName);
}
return shortNamespaces;
Expand Down
24 changes: 20 additions & 4 deletions test/DynamoCoreTests/NodeToCodeTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -885,14 +885,30 @@ public void TestPropertyToStaticPropertyInvocation()

var rhs = result.AstNodes.Skip(1).Select(b => (b as BinaryExpressionNode).RightNode.ToString().Contains("ValueContainer.SomeValue"));
Assert.IsTrue(rhs.All(r => r));
}

CurrentDynamoModel.ForceRun();
[Test]
public void NonUniqueNamespaceConflict_on_node2code_doesNotCrash()
{
string libraryPath = "FFITarget.dll";
if (!CurrentDynamoModel.EngineController.LibraryServices.IsLibraryLoaded(libraryPath))
{
CurrentDynamoModel.EngineController.LibraryServices.ImportLibrary(libraryPath);
}

var cbn = CurrentDynamoModel.CurrentWorkspace.Nodes.OfType<CodeBlockNodeModel>().FirstOrDefault();
// this graph contains a single "FFITarget.B.DupTargetTest" node that conflicts non-uniquely with namespace "FFITarget.C.B.DupTargetTest"
OpenModel(@"core\node2code\NonUniqueNamespaceConflict_throwsNodeWarning.dyn");
var nodes = CurrentDynamoModel.CurrentWorkspace.Nodes;
SelectAll(nodes);
var command = new DynamoModel.ConvertNodesToCodeCommand();
CurrentDynamoModel.ExecuteCommand(command);

nodes = CurrentDynamoModel.CurrentWorkspace.Nodes;
var cbn = nodes.OfType<CodeBlockNodeModel>().FirstOrDefault();
Assert.IsNotNull(cbn);

var guid = cbn.GUID.ToString();
AssertPreviewValue(guid, 23);
var error = "Multiple definitions for 'FFITarget.B.DupTargetTest' are found as FFITarget.C.B.DupTargetTest, FFITarget.B.DupTargetTest";
Assert.IsTrue(cbn.ToolTipText.Contains(error));
}


Expand Down
19 changes: 2 additions & 17 deletions test/Engine/ProtoTest/DebugTests/NamespaceConflictTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ class NamespaceConflictTest : ProtoTestBase
{
[Test]
[Category("Trace")]
[Category("Failure")]
public void DupImportTest()
{
var mirror = thisTest.RunScriptSource(
Expand All @@ -23,10 +22,9 @@ public void DupImportTest()
bO = b.Foo();
"
);
// Tracked by http://adsk-oss.myjetbrains.com/youtrack/issue/MAGN-1947
string defectID = "MAGN-1947 IntegrationTests.NamespaceConflictTest.DupImportTest";
thisTest.VerifyBuildWarningCount(ProtoCore.BuildData.WarningID.MultipleSymbolFoundFromName, 1);
thisTest.Verify("aO", 1);
thisTest.Verify("bO", 2);
thisTest.Verify("bO", null);

}

Expand Down Expand Up @@ -54,18 +52,5 @@ public void DupImportTestNamespaceConflict02()
thisTest.VerifyBuildWarningCount(ProtoCore.BuildData.WarningID.MultipleSymbolFoundFromName, 1);
thisTest.Verify("p", null);
}


[Test]
[Category("Trace")]
public void DupImportTestNeg()
{
var mirror = thisTest.RunScriptSource(
@"import(""FFITarget.dll"");
a = DupTargetTest.DupTargetTest();
aO = a.Foo();
"
);
}
}
}
34 changes: 12 additions & 22 deletions test/Engine/ProtoTest/FFITests/CSFFITest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1290,62 +1290,52 @@ public void TestNamespacePartialResolution03()
}

[Test]
[Category("Failure")]
public void TestNamespaceClassResolution()
{
// Tracked by http://adsk-oss.myjetbrains.com/youtrack/issue/MAGN-1947
string code =
@"import(""FFITarget.dll"");
import(""BuiltIn.ds"");
x = 1..2;
x = 1..2;
Xo = x[0];
aDup = A.DupTargetTest(x);
aDup = A.DupTargetTest.DupTargetTest(x);
aReadback = aDup.Prop[0];
bDup = B.DupTargetTest(x);
bReadback = bDup.Prop[1];
check = List.Equals(aDup.Prop,bDup.Prop);";
bDup = B.DupTargetTest.DupTargetTest(x);
bReadback = bDup.Prop[1];";

thisTest.RunScriptSource(code);
thisTest.Verify("check", true);
thisTest.Verify("Xo", 1);

thisTest.Verify("aReadback", 1);
thisTest.Verify("bReadback", 2);
thisTest.Verify("bReadback", null);

TestFrameWork.VerifyBuildWarning(ProtoCore.BuildData.WarningID.MultipleSymbolFound);
string[] classes = thisTest.GetAllMatchingClasses("DupTargetTest");
Assert.True(classes.Length > 1, "More than one implementation of DupTargetTest class expected");
}

[Test]
[Category("Failure")]
public void TestSubNamespaceClassResolution()
{
// Tracked by http://adsk-oss.myjetbrains.com/youtrack/issue/MAGN-1947
string code =
@"import(""FFITarget.dll"");
import(""BuiltIn.ds"");
aDup = A.DupTargetTest(0);
aDup = A.DupTargetTest.DupTargetTest(0);
aReadback = aDup.Prop;
bDup = B.DupTargetTest(1); //This should match exactly BClass.DupTargetTest
bDup = B.DupTargetTest.DupTargetTest(1);
bReadback = bDup.Prop;
cDup = C.B.DupTargetTest(2);
cReadback = cDup.Prop;
check = List.Equals(aDup.Prop,bDup.Prop);
check = List.Equals(bDup.Prop,cDup.Prop);
cDup = C.B.DupTargetTest.DupTargetTest(2);
cReadback = cDup.Prop;
";
string err = "MAGN-1947 IntegrationTests.NamespaceConflictTest.DupImportTest";
thisTest.RunScriptSource(code, err);
thisTest.Verify("check", true);
thisTest.RunScriptSource(code);
thisTest.Verify("aReadback", 0);
thisTest.Verify("bReadback", 1);
thisTest.Verify("bDup", null);
thisTest.Verify("bReadback", null);
thisTest.Verify("cReadback", 2);

TestFrameWork.VerifyBuildWarning(ProtoCore.BuildData.WarningID.MultipleSymbolFound);
Expand Down
74 changes: 74 additions & 0 deletions test/core/node2code/NonUniqueNamespaceConflict_throwsNodeWarning.dyn
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
{
"Uuid": "d975bf5b-a5ef-45af-b39b-ae5704b9b814",
"IsCustomNode": false,
"Description": null,
"Name": "NonUniqueNamespaceConflict_throwsNodeWarning",
"ElementResolver": {
"ResolutionMap": {}
},
"Inputs": [],
"Outputs": [],
"Nodes": [
{
"ConcreteType": "Dynamo.Graph.Nodes.ZeroTouch.DSFunction, DynamoCore",
"NodeType": "FunctionNode",
"FunctionSignature": "FFITarget.B.DupTargetTest.DupTargetTest",
"Id": "ebb33bb13f4143d5b9ba64c8c421f1c3",
"Inputs": [],
"Outputs": [
{
"Id": "70a9cb93033a431d88de21406cef4241",
"Name": "DupTargetTest",
"Description": "DupTargetTest",
"UsingDefaultValue": false,
"Level": 2,
"UseLevels": false,
"KeepListStructure": false
}
],
"Replication": "Auto",
"Description": "DupTargetTest.DupTargetTest ( ): DupTargetTest"
}
],
"Connectors": [],
"Dependencies": [],
"Bindings": [],
"View": {
"Dynamo": {
"ScaleFactor": 1.0,
"HasRunWithoutCrash": true,
"IsVisibleInDynamoLibrary": true,
"Version": "2.1.0.6876",
"RunType": "Automatic",
"RunPeriod": "1000"
},
"Camera": {
"Name": "Background Preview",
"EyeX": -17.0,
"EyeY": 24.0,
"EyeZ": 50.0,
"LookX": 12.0,
"LookY": -13.0,
"LookZ": -58.0,
"UpX": 0.0,
"UpY": 1.0,
"UpZ": 0.0
},
"NodeViews": [
{
"ShowGeometry": true,
"Name": "DupTargetTest.DupTargetTest",
"Id": "ebb33bb13f4143d5b9ba64c8c421f1c3",
"IsSetAsInput": false,
"IsSetAsOutput": false,
"Excluded": false,
"X": 301.333333333333,
"Y": 250.666666666667
}
],
"Annotations": [],
"X": 0.0,
"Y": 0.0,
"Zoom": 1.0
}
}

0 comments on commit 896e0b2

Please sign in to comment.