-
Notifications
You must be signed in to change notification settings - Fork 11
Improve diff output #163
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
Merged
Merged
Improve diff output #163
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| using KustoSchemaTools.Changes; | ||
| using KustoSchemaTools.Model; | ||
|
|
||
| namespace KustoSchemaTools.Tests.Changes | ||
| { | ||
| public class ClusterGroupingTests | ||
| { | ||
| [Fact] | ||
| public void BuildClusterFingerprint_IdenticalChanges_ProduceSameFingerprint() | ||
| { | ||
| var changes1 = CreateSampleChanges("## Table1\nSome diff"); | ||
| var changes2 = CreateSampleChanges("## Table1\nSome diff"); | ||
| var comments = new List<Comment>(); | ||
|
|
||
| var fp1 = KustoSchemaHandler<Database>.BuildClusterFingerprint(changes1, comments, true); | ||
| var fp2 = KustoSchemaHandler<Database>.BuildClusterFingerprint(changes2, comments, true); | ||
|
|
||
| Assert.Equal(fp1, fp2); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void BuildClusterFingerprint_DifferentMarkdown_ProduceDifferentFingerprints() | ||
| { | ||
| var changes1 = CreateSampleChanges("## Table1\nDiff A"); | ||
| var changes2 = CreateSampleChanges("## Table1\nDiff B"); | ||
| var comments = new List<Comment>(); | ||
|
|
||
| var fp1 = KustoSchemaHandler<Database>.BuildClusterFingerprint(changes1, comments, true); | ||
| var fp2 = KustoSchemaHandler<Database>.BuildClusterFingerprint(changes2, comments, true); | ||
|
|
||
| Assert.NotEqual(fp1, fp2); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void BuildClusterFingerprint_DifferentValidity_ProduceDifferentFingerprints() | ||
| { | ||
| var changes = CreateSampleChanges("## Table1\nSame diff"); | ||
| var comments = new List<Comment>(); | ||
|
|
||
| var fp1 = KustoSchemaHandler<Database>.BuildClusterFingerprint(changes, comments, true); | ||
| var fp2 = KustoSchemaHandler<Database>.BuildClusterFingerprint(changes, comments, false); | ||
|
|
||
| Assert.NotEqual(fp1, fp2); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void BuildClusterFingerprint_DifferentComments_ProduceDifferentFingerprints() | ||
| { | ||
| var changes = CreateSampleChanges("## Table1\nSame diff"); | ||
| var comments1 = new List<Comment> | ||
| { | ||
| new Comment { Kind = CommentKind.Warning, Text = "Warning 1", FailsRollout = false } | ||
| }; | ||
| var comments2 = new List<Comment> | ||
| { | ||
| new Comment { Kind = CommentKind.Caution, Text = "Caution 1", FailsRollout = true } | ||
| }; | ||
|
|
||
| var fp1 = KustoSchemaHandler<Database>.BuildClusterFingerprint(changes, comments1, true); | ||
| var fp2 = KustoSchemaHandler<Database>.BuildClusterFingerprint(changes, comments2, true); | ||
|
|
||
| Assert.NotEqual(fp1, fp2); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void BuildClusterFingerprint_EmptyChanges_ProduceSameFingerprint() | ||
| { | ||
| var changes = new List<IChange>(); | ||
| var comments = new List<Comment>(); | ||
|
|
||
| var fp1 = KustoSchemaHandler<Database>.BuildClusterFingerprint(changes, comments, true); | ||
| var fp2 = KustoSchemaHandler<Database>.BuildClusterFingerprint(changes, comments, true); | ||
|
|
||
| Assert.Equal(fp1, fp2); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void BuildClusterFingerprint_CommentsInDifferentOrder_ProduceSameFingerprint() | ||
| { | ||
| var changes = CreateSampleChanges("## Table1\nSame diff"); | ||
| var comments1 = new List<Comment> | ||
| { | ||
| new Comment { Kind = CommentKind.Warning, Text = "A", FailsRollout = false }, | ||
| new Comment { Kind = CommentKind.Note, Text = "B", FailsRollout = false } | ||
| }; | ||
| var comments2 = new List<Comment> | ||
| { | ||
| new Comment { Kind = CommentKind.Note, Text = "B", FailsRollout = false }, | ||
| new Comment { Kind = CommentKind.Warning, Text = "A", FailsRollout = false } | ||
| }; | ||
|
|
||
| var fp1 = KustoSchemaHandler<Database>.BuildClusterFingerprint(changes, comments1, true); | ||
| var fp2 = KustoSchemaHandler<Database>.BuildClusterFingerprint(changes, comments2, true); | ||
|
|
||
| Assert.Equal(fp1, fp2); | ||
| } | ||
|
|
||
| private static List<IChange> CreateSampleChanges(string markdown) | ||
| { | ||
| return new List<IChange> | ||
| { | ||
| new FakeChange(markdown) | ||
| }; | ||
| } | ||
|
|
||
| private class FakeChange : IChange | ||
| { | ||
| public FakeChange(string markdown) | ||
| { | ||
| Markdown = markdown; | ||
| } | ||
|
|
||
| public string EntityType => "Test"; | ||
| public string Entity => "TestEntity"; | ||
| public List<DatabaseScriptContainer> Scripts => new List<DatabaseScriptContainer>(); | ||
| public string Markdown { get; } | ||
| public Comment Comment { get; set; } | ||
| } | ||
| } | ||
| } |
185 changes: 185 additions & 0 deletions
185
KustoSchemaTools.Tests/Changes/ColumnDiffHelperTests.cs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,185 @@ | ||
| using KustoSchemaTools.Changes; | ||
|
|
||
| namespace KustoSchemaTools.Tests.Changes | ||
| { | ||
| public class ColumnDiffHelperTests | ||
| { | ||
| [Fact] | ||
| public void BuildColumnDiff_WithAddedColumns_ShowsAdditions() | ||
| { | ||
| var oldColumns = new Dictionary<string, string> | ||
| { | ||
| { "id", "string" }, | ||
| { "timestamp", "datetime" } | ||
| }; | ||
| var newColumns = new Dictionary<string, string> | ||
| { | ||
| { "id", "string" }, | ||
| { "timestamp", "datetime" }, | ||
| { "new_col", "long" } | ||
| }; | ||
|
|
||
| var result = ColumnDiffHelper.BuildColumnDiff(oldColumns, newColumns); | ||
|
|
||
| Assert.NotNull(result); | ||
| Assert.Contains("+ new_col: long", result); | ||
| Assert.DoesNotContain("id", result); | ||
| Assert.DoesNotContain("timestamp", result); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void BuildColumnDiff_WithTypeChange_ShowsTypeChange() | ||
| { | ||
| var oldColumns = new Dictionary<string, string> | ||
| { | ||
| { "id", "string" }, | ||
| { "count", "int" } | ||
| }; | ||
| var newColumns = new Dictionary<string, string> | ||
| { | ||
| { "id", "string" }, | ||
| { "count", "long" } | ||
| }; | ||
|
|
||
| var result = ColumnDiffHelper.BuildColumnDiff(oldColumns, newColumns); | ||
|
|
||
| Assert.NotNull(result); | ||
| Assert.Contains("! count: int → long", result); | ||
| Assert.DoesNotContain("id", result); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void BuildColumnDiff_WithRemovedColumns_ShowsInformationalNote() | ||
| { | ||
| var oldColumns = new Dictionary<string, string> | ||
| { | ||
| { "id", "string" }, | ||
| { "old_col", "string" }, | ||
| { "timestamp", "datetime" } | ||
| }; | ||
| var newColumns = new Dictionary<string, string> | ||
| { | ||
| { "id", "string" }, | ||
| { "timestamp", "datetime" } | ||
| }; | ||
|
|
||
| var result = ColumnDiffHelper.BuildColumnDiff(oldColumns, newColumns); | ||
|
|
||
| Assert.NotNull(result); | ||
| Assert.Contains("old_col: string (live only)", result); | ||
| Assert.Contains(".create-merge", result); | ||
| Assert.DoesNotContain("+ ", result); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void BuildColumnDiff_WithNoChanges_ReturnsNull() | ||
| { | ||
| var oldColumns = new Dictionary<string, string> | ||
| { | ||
| { "id", "string" }, | ||
| { "timestamp", "datetime" } | ||
| }; | ||
| var newColumns = new Dictionary<string, string> | ||
| { | ||
| { "id", "string" }, | ||
| { "timestamp", "datetime" } | ||
| }; | ||
|
|
||
| var result = ColumnDiffHelper.BuildColumnDiff(oldColumns, newColumns); | ||
|
|
||
| Assert.Null(result); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void BuildColumnDiff_WithNullOldColumns_TreatsAsNewTable() | ||
| { | ||
| var newColumns = new Dictionary<string, string> | ||
| { | ||
| { "id", "string" }, | ||
| { "timestamp", "datetime" } | ||
| }; | ||
|
|
||
| var result = ColumnDiffHelper.BuildColumnDiff(null, newColumns); | ||
|
|
||
| Assert.NotNull(result); | ||
| Assert.Contains("+ id: string", result); | ||
| Assert.Contains("+ timestamp: datetime", result); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void BuildColumnDiff_WithNullNewColumns_ReturnsNull() | ||
| { | ||
| var oldColumns = new Dictionary<string, string> | ||
| { | ||
| { "id", "string" } | ||
| }; | ||
|
|
||
| var result = ColumnDiffHelper.BuildColumnDiff(oldColumns, null); | ||
|
|
||
| Assert.Null(result); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void BuildColumnDiff_WithMixedChanges_ShowsAllCategories() | ||
| { | ||
| var oldColumns = new Dictionary<string, string> | ||
| { | ||
| { "id", "string" }, | ||
| { "count", "int" }, | ||
| { "removed_col", "string" } | ||
| }; | ||
| var newColumns = new Dictionary<string, string> | ||
| { | ||
| { "id", "string" }, | ||
| { "count", "long" }, | ||
| { "new_col", "datetime" } | ||
| }; | ||
|
|
||
| var result = ColumnDiffHelper.BuildColumnDiff(oldColumns, newColumns); | ||
|
|
||
| Assert.NotNull(result); | ||
| Assert.Contains("+ new_col: datetime", result); | ||
| Assert.Contains("! count: int → long", result); | ||
| Assert.Contains("removed_col: string (live only)", result); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void BuildColumnDiff_CaseInsensitiveTypeComparison_IgnoresCaseDifference() | ||
| { | ||
| var oldColumns = new Dictionary<string, string> | ||
| { | ||
| { "id", "String" } | ||
| }; | ||
| var newColumns = new Dictionary<string, string> | ||
| { | ||
| { "id", "string" } | ||
| }; | ||
|
|
||
| var result = ColumnDiffHelper.BuildColumnDiff(oldColumns, newColumns); | ||
|
|
||
| Assert.Null(result); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void BuildColumnDiff_ReorderOnly_ReturnsNull() | ||
| { | ||
| var oldColumns = new Dictionary<string, string> | ||
| { | ||
| { "id", "string" }, | ||
| { "timestamp", "datetime" }, | ||
| { "count", "long" } | ||
| }; | ||
| // Same columns, different insertion order in the dictionary | ||
| var newColumns = new Dictionary<string, string> | ||
| { | ||
| { "count", "long" }, | ||
| { "id", "string" }, | ||
| { "timestamp", "datetime" } | ||
| }; | ||
|
|
||
| var result = ColumnDiffHelper.BuildColumnDiff(oldColumns, newColumns); | ||
|
|
||
| Assert.Null(result); | ||
| } | ||
| } | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| using KustoSchemaTools.Model; | ||
| using System.Text; | ||
|
|
||
| namespace KustoSchemaTools.Changes | ||
| { | ||
| /// <summary> | ||
| /// Generates a human-readable column-level diff for table schema changes. | ||
| /// Instead of showing full .create-merge table lines, shows only the columns | ||
| /// that were added or had their type changed. | ||
| /// </summary> | ||
| public static class ColumnDiffHelper | ||
| { | ||
| /// <summary> | ||
| /// Builds a column-level diff between two sets of columns. | ||
| /// </summary> | ||
| /// <param name="oldColumns">Columns from the live cluster state (may be null for new tables).</param> | ||
| /// <param name="newColumns">Columns from the desired YAML state.</param> | ||
| /// <returns> | ||
| /// A formatted diff string showing added columns and type changes, | ||
| /// or null if there are no meaningful column differences. | ||
| /// </returns> | ||
| public static string? BuildColumnDiff(Dictionary<string, string>? oldColumns, Dictionary<string, string>? newColumns) | ||
| { | ||
| if (newColumns == null) return null; | ||
| oldColumns ??= new Dictionary<string, string>(); | ||
|
|
||
| var added = newColumns | ||
| .Where(c => !oldColumns.ContainsKey(c.Key)) | ||
| .ToList(); | ||
|
|
||
| var typeChanged = newColumns | ||
| .Where(c => oldColumns.ContainsKey(c.Key) && !string.Equals(oldColumns[c.Key], c.Value, StringComparison.OrdinalIgnoreCase)) | ||
| .Select(c => new { Name = c.Key, OldType = oldColumns[c.Key], NewType = c.Value }) | ||
| .ToList(); | ||
|
|
||
| var removedFromYaml = oldColumns | ||
| .Where(c => !newColumns.ContainsKey(c.Key)) | ||
| .ToList(); | ||
|
|
||
| if (!added.Any() && !typeChanged.Any() && !removedFromYaml.Any()) | ||
| return null; | ||
|
|
||
| var sb = new StringBuilder(); | ||
| sb.AppendLine("```diff"); | ||
|
|
||
| foreach (var col in added) | ||
| { | ||
| sb.AppendLine($"+ {col.Key}: {col.Value}"); | ||
| } | ||
|
|
||
| foreach (var col in typeChanged) | ||
| { | ||
| sb.AppendLine($"! {col.Name}: {col.OldType} → {col.NewType}"); | ||
| } | ||
|
|
||
| if (removedFromYaml.Any()) | ||
| { | ||
| sb.AppendLine("```"); | ||
| sb.AppendLine(); | ||
| sb.AppendLine("> **Note**: The following columns exist in the live cluster but not in YAML."); | ||
| sb.AppendLine("> `.create-merge` does not remove columns — they will remain on the table."); | ||
| sb.AppendLine(); | ||
| sb.AppendLine("```"); | ||
| foreach (var col in removedFromYaml) | ||
| { | ||
| sb.AppendLine($" {col.Key}: {col.Value} (live only)"); | ||
| } | ||
| } | ||
|
|
||
| sb.AppendLine("```"); | ||
| return sb.ToString(); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
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.
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.
BuildColumnDiff enumerates
newColumns/oldColumnsdirectly when renderingadded,typeChanged, andremovedFromYaml. Since these areDictionaryinstances, enumeration order can vary depending on how the dictionaries were constructed, which can make the rendered diff (and cluster grouping fingerprint) non-deterministic. Sorting by column name before rendering (and sorting removed columns) would make the output stable across runs/clusters.