Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: Generate Ignore.xml #18

Closed
wants to merge 4 commits into from
Closed

feat: Generate Ignore.xml #18

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7db5daa
feat: Generate Ingnore.xml
workgroupengineering Jan 4, 2023
c483743
fix: tests
workgroupengineering Jan 5, 2023
e67d5c2
feat: Allowed generate GitHub Comment on PR with changes
workgroupengineering Jan 5, 2023
4edfc56
fix: Address review
workgroupengineering Jan 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/Uno.PackageDiff.Tests/Given_ReportAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public void When_Empty_IgnoreSet()

var res = AssemblyComparer.CompareTypes(context.BaseAssembly, context.TargetAssembly);

Assert.IsFalse(ReportAnalyzer.GenerateReport(StreamWriter.Null, res, context.IgnoreSet));
Assert.IsFalse(ReportAnalyzer.GenerateReport(default, res, context.IgnoreSet));
}

[TestMethod]
Expand All @@ -34,7 +34,7 @@ public void When_IgnoreProperty()
var res = AssemblyComparer.CompareTypes(context.BaseAssembly, context.TargetAssembly);

Assert.IsNotNull(context.IgnoreSet);
Assert.IsTrue(ReportAnalyzer.GenerateReport(StreamWriter.Null, res, context.IgnoreSet));
Assert.IsTrue(ReportAnalyzer.GenerateReport(default, res, context.IgnoreSet));
}

[TestMethod]
Expand All @@ -45,7 +45,7 @@ public void When_Ignore_All_Changes_To_Type()
var comparison = AssemblyComparer.CompareTypes(context.BaseAssembly, context.TargetAssembly);

Assert.IsNotNull(context.IgnoreSet);
Assert.IsFalse(ReportAnalyzer.GenerateReport(StreamWriter.Null, comparison, context.IgnoreSet));
Assert.IsFalse(ReportAnalyzer.GenerateReport(default, comparison, context.IgnoreSet));
}

[TestMethod]
Expand All @@ -56,7 +56,7 @@ public void When_Ignore_All_Changes_With_Major_Minor_Only()
var comparison = AssemblyComparer.CompareTypes(context.BaseAssembly, context.TargetAssembly);

Assert.IsNotNull(context.IgnoreSet);
Assert.IsFalse(ReportAnalyzer.GenerateReport(StreamWriter.Null, comparison, context.IgnoreSet));
Assert.IsFalse(ReportAnalyzer.GenerateReport(default, comparison, context.IgnoreSet));
}

[TestMethod]
Expand All @@ -67,7 +67,7 @@ public void When_Ignore_All_Changes_With_Major_Only()
var comparison = AssemblyComparer.CompareTypes(context.BaseAssembly, context.TargetAssembly);

Assert.IsNotNull(context.IgnoreSet);
Assert.IsFalse(ReportAnalyzer.GenerateReport(StreamWriter.Null, comparison, context.IgnoreSet));
Assert.IsFalse(ReportAnalyzer.GenerateReport(default, comparison, context.IgnoreSet));
}
}
}
33 changes: 33 additions & 0 deletions src/Uno.PackageDiff/GitHubClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
using System;
using System.Text;
using System.Threading.Tasks;
using Newtonsoft.Json;
using System.Web;

namespace Uno.PackageDiff
{
internal class GitHubClient
Xiaoy312 marked this conversation as resolved.
Show resolved Hide resolved
{
public static async Task PostPRCommentsAsync(string githubPAT, string sourceRepository, string githubPRid, string comment)
{
var uri = new Uri(sourceRepository);
var path = uri.GetComponents(UriComponents.Path, UriFormat.UriEscaped).Replace(".git", "");

var bodyContent = $"{{ \"body\": \"{HttpUtility.JavaScriptStringEncode(comment)}\" }}";

await PostDocument(new Uri($"https://api.github.com/repos/{path}/issues/{githubPRid}/comments"), githubPAT, bodyContent);
}

private static async Task<dynamic> PostDocument(Uri contentUri, string githubPAT, string document)
{
var wc = new System.Net.WebClient();
wc.Headers.Add("User-agent", "uno-nv-sync");
wc.Headers.Add("Authorization", $"token {githubPAT}");
wc.Encoding = UTF8Encoding.UTF8;

var result = await wc.UploadStringTaskAsync(contentUri, "POST", document);

return JsonConvert.DeserializeObject(result);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dynamic seem like a worse idea, than just returning the string
but, if you arent using the result pass debugging stage
it is better to just let this be a Task of nothing

}
}
}
43 changes: 43 additions & 0 deletions src/Uno.PackageDiff/IDiffWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
using System;
using NuGet.Versioning;
using Mono.Cecil;

namespace Uno.PackageDiff
{
internal interface IDiffWriter : IDisposable
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this contract looks highly tied to xml syntax (WriteXyzStart, WriteXyzEnd), and md formatting (XyzStart that takes in an int).
it should instead accept the array of missing types, and (containing types + their missing members: properties,fields,method and events)
or anything more context-relevant

{
void WriteAssemblyName(string v);

void WriteHeader((string Package, NuGetVersion Version) source, (string Package, NuGetVersion Version) target);

void WritePlatform(string platform);

void WriteMissingTypesStart(int invalidTypesCount);
void WriteMissingType(TypeDefinition typeDef, bool isIgnored);
void WriteMissingTypesEnd();

void WriteMissingPropertiesStart(int invalidPropertiesCount);
void WriteMissingPropertiesForTypeStart(string typeName);
void WriteMissingProperty(PropertyDefinition propertyDef, bool isIgnored);
void WriteMissingPropertiesForTypeEnd();
void WriteMissingPropertiesEnd();

void WriteMissingFieldsStart(int invalidFieldsCount);
void WriteMissingFieldForTypeStart(string typeName);
void WriteMissingField(FieldDefinition fieldDef, bool isIgnored);
void WriteMissingFieldForTypeEnd();
void WriteMissingFieldsEnd();

void WriteMissingMethodsStart(int invalidMethodsCount);
void WriteMissingMethodsForTypeStart(string typeName);
void WriteMissingMethod(MethodDefinition methodDef, bool isIgnored);
void WriteMissingMethodsForTypeEnd();
void WriteMissingMethodsEnd();

void WriteMissingEventsStart(int invalidEventsCount);
void WriteMissingEventsForTypeStart(string typeName);
void WriteMissingEvent(EventDefinition eventDef, bool isIgnored);
void WriteMissingEventsForTypeEnd();
void WriteMissingEventsEnd();
}
}
79 changes: 66 additions & 13 deletions src/Uno.PackageDiff/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,77 @@

namespace Uno.PackageDiff
{
class Program
class Program
{
private static readonly PackageSource NuGetOrgSource = new PackageSource("https://api.nuget.org/v3/index.json");

static async Task<int> Main(string[] args)
{
{
string sourceArgument = null;
string targetArgument = null;
string outputFile = null;
string diffIgnoreFile = null;
string outputType = null;
string githubPAT = null;
string sourceRepository = null;
string githubPRid = null;

var p = new OptionSet() {
{ "base=", s => sourceArgument = s },
{ "other=", s => targetArgument = s },
{ "outfile=", s => outputFile = s },
{ "diffignore=", s => diffIgnoreFile = s },
{ "outtype=", s => outputType = s},

//
// GitHub PR comments related
//
{ "github-pat=", s => githubPAT = s },
{ "source-repository=", s => sourceRepository = s },
{ "github-pr-id=", s => githubPRid = s }
};

p.Parse(args);

IDiffWriter writer = null;
if(string.IsNullOrEmpty(outputType))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the if-else can be extracted to a separate method:

using (var writer = BuildWriter(outtype, /* github params... */))

{
writer = new Writers.MDDiffWriter(outputFile);
}
else
{
var types = outputType.Split(",", StringSplitOptions.RemoveEmptyEntries);
if(types.Length == 0)
{
writer = new Writers.MDDiffWriter(outputFile);
}
else
{
var composite = new Writers.CompositeWriter();
foreach(var type in types)
{
if(string.Equals("md", type, StringComparison.OrdinalIgnoreCase))
{
composite.Add(new Writers.MDDiffWriter(outputFile));
}
else if(string.Equals("diff", type, StringComparison.OrdinalIgnoreCase))
{
composite.Add(new Writers.XmlIngnoreWriter(outputFile));
}
else if(string.Equals("github", type, StringComparison.OrdinalIgnoreCase))
{
composite.Add(new Writers.GitHubWriter(githubPAT, sourceRepository, githubPRid));
}
}
if(composite.Count == 0)
{
Console.WriteLine($"Error : Invalid outtype {outputType}.");
return 1;
}
Comment on lines +68 to +87
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a combination of valid and invalid outtype will slip through here... ex: "outtype=diff,asd"

you should add a final else: else { Console.WriteLine($"Error: Invalid outtype: {type}"); }
and change the final if to composite.Count != types.Length

writer = composite;
}
}

var source = await GetPackage(sourceArgument);
var target = await GetPackage(targetArgument);

Expand All @@ -54,18 +105,20 @@ join targetAssemblyPath in targetPlatforms on Path.GetFileName(sourceAssemblyPat
Target = targetAssemblyPath
};


var ignoreSet = DiffIgnore.ParseDiffIgnore(diffIgnoreFile, source.nuspecReader.GetVersion().ToString());

bool differences = false;
using (var writer = new StreamWriter(outputFile))
using(writer)
{
writer.WriteLine($"Comparison report for {source.nuspecReader.GetId()} **{source.nuspecReader.GetVersion()}** with {target.nuspecReader.GetId()} **{target.nuspecReader.GetVersion()}**");
writer.WriteHeader(source: (source.nuspecReader.GetId(), source.nuspecReader.GetVersion())
, target: (target.nuspecReader.GetId(), target.nuspecReader.GetVersion()));

foreach(var platform in platformsFiles)
{
writer.WriteLine($"# {platform.Platform} Platform");
writer.WritePlatform(platform.Platform);

foreach (var sourceFile in Directory.GetFiles(platform.Source, "*.dll"))
foreach(var sourceFile in Directory.GetFiles(platform.Source, "*.dll"))
{
var targetFile = Path.Combine(platform.Target, Path.GetFileName(sourceFile));

Expand All @@ -92,7 +145,7 @@ join targetAssemblyPath in targetPlatforms on Path.GetFileName(sourceAssemblyPat
var withDifferences = differences ? ", with differences" : "";
Console.WriteLine($"Done comparing{withDifferences}.");

if (differences)
if(differences)
{
Console.WriteLine(@"Error : Build failed with unexpected differences. Modifications to the public API introduce binary breaking changes and should be avoided.");
Console.WriteLine("If these modifications were expected and intended, see https://github.com/nventive/Uno.PackageDiff#how-to-provide-an-ignore-set on how to ignore them.");
Expand All @@ -107,12 +160,12 @@ join targetAssemblyPath in targetPlatforms on Path.GetFileName(sourceAssemblyPat
}
}

public static bool CompareAssemblies(StreamWriter writer, string sourceFile, string targetFile, IgnoreSet ignoreSet)
public static bool CompareAssemblies(IDiffWriter writer, string sourceFile, string targetFile, IgnoreSet ignoreSet)
{
using(var source = ReadModule(sourceFile))
using(var target = ReadModule(targetFile))
{
writer.WriteLine($"## {Path.GetFileName(sourceFile)}");
writer.WriteAssemblyName(Path.GetFileName(sourceFile));
var results = AssemblyComparer.CompareTypes(source, target);

return ReportAnalyzer.GenerateReport(writer, results, ignoreSet);
Expand Down Expand Up @@ -146,7 +199,7 @@ private static AssemblyDefinition ReadModule(string path)
.Where(v => !v.Version.IsPrerelease)
.FirstOrDefault();

if (latestStable != null)
if(latestStable != null)
{
var packageId = packagePath.ToLowerInvariant();
var version = latestStable.Version.ToNormalizedString().ToLowerInvariant();
Expand Down Expand Up @@ -182,9 +235,9 @@ private static (string path, NuspecReader nuspecReader) UnpackArchive(string pac
Directory.CreateDirectory(path);

Console.WriteLine($"Extracting {packagePath} -> {path}");
using (var file = File.OpenRead(packagePath))
using(var file = File.OpenRead(packagePath))
{
using (var archive = new ZipArchive(file, ZipArchiveMode.Read))
using(var archive = new ZipArchive(file, ZipArchiveMode.Read))
{
archive.ExtractToDirectory(path);
}
Expand All @@ -200,5 +253,5 @@ private static (string path, NuspecReader nuspecReader) UnpackArchive(string pac
}
}

}
}
}
Loading