T1.1 Python 2 Code Migration - #10850

On Jul 20, 2020, at 8:56 AM, Martin Misol Monzo ***@***.***> wrote: @mmisol commented on this pull request. Just to move things up a little, I commented the things I feel still need addressing. I'll summarize here: Delete third unit test (unless there is something I overlooked, LMK) Add new libraries to license files Fix bug related to unsaved code changes not showing in the migrator In test/DynamoCoreTests/PythonMigrationAssistantTests.cs: > + + [Test] + public void MigrationWillNotChangeCodeAfterBeingConverted() + { + // Arrange + var originalPython2Code = @"for x in xrange(100): continue"; + var expectedPython3CodeAfterMigration = @"for x in range(100): continue"; + + // Act + var migratedScript = ScriptMigrator.MigrateCode(originalPython2Code); + var migratedScript2 = ScriptMigrator.MigrateCode(migratedScript); + + // Assert + Assert.AreEqual(expectedPython3CodeAfterMigration, migratedScript); + Assert.AreEqual(expectedPython3CodeAfterMigration, migratedScript2); + } @radumg Since you mentioned idempotency let's get mathematical :) The second test shows that x = f(x) when x doesn't have Python 2 in it. The third tests shows that y = f(f(y)) with a y with the same characteristics as x in the previous test. The second test is already showing idempotency, therefore my argument to delete this third test. In test/DynamoCoreTests/PythonMigrationAssistantTests.cs: > + [Test] + public void MigrationWillNotChangeCodeAfterBeingConverted() + { + // Arrange + var originalPython2Code = @"for x in xrange(100): continue"; + var expectedPython3CodeAfterMigration = @"for x in range(100): continue"; + + // Act + var migratedScript = ScriptMigrator.MigrateCode(originalPython2Code); + var migratedScript2 = ScriptMigrator.MigrateCode(migratedScript); + + // Assert + Assert.AreEqual(expectedPython3CodeAfterMigration, migratedScript); + Assert.AreEqual(expectedPython3CodeAfterMigration, migratedScript2); + } + } Brought this to standup the other day and I didn't get any remarks from Mike. Unless he comments otherwise, let's assume we don't need this for now, for the sake of unblocking this PR. In src/PythonMigrationViewExtension/packages.config: > @@ -0,0 +1,7 @@ +<?xml version="1.0" encoding="utf-8"?> +<packages> + <package id="DiffPlex" version="1.6.3" targetFramework="net48" /> + <package id="DiffPlex.Wpf" version="1.1.1" targetFramework="net48" /> @SHKnudsen Please address this. In src/Libraries/PythonNodeModelsWpf/ScriptEditorWindow.xaml.cs: > @@ -167,6 +167,13 @@ private void OnRunClicked(object sender, RoutedEventArgs e) } } + private void OnMigrationAssistantClicked(object sender, RoutedEventArgs e) + { + if (nodeModel == null) + throw new NullReferenceException(nameof(nodeModel)); + + nodeModel.RequestCodeMigration(e); + } Please address this bug before merging @SHKnudsen — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.