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.

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

Rev yaml-test-suite submodule and fix Unix build #631

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Set default behavior to automatically normalize line endings.
* text=auto

# Force bash scripts to always use lf line endings so that if a repo is accessed
# in Unix via a file share from Windows, the scripts will work.
*.sh text eol=lf

# Likewise, force cmd and batch scripts to always use crlf
*.cmd text eol=crlf
*.bat text eol=crlf

*.cs text=auto diff=csharp

*.csproj text=auto
*.sln text=auto eol=crlf
2 changes: 1 addition & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@
[submodule "yaml-test-suite"]
path = YamlDotNet.Test/yaml-test-suite
url = https://github.com/yaml/yaml-test-suite
branch = data-2020-02-11
branch = data-2020-08-01
6 changes: 3 additions & 3 deletions YamlDotNet.Test/Serialization/SerializationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1206,10 +1206,10 @@ public void DontIgnoreExtraPropertiesIfWanted()
Assert.IsType<YamlException>(actual);
((YamlException)actual).Start.Column.Should().Be(1);
((YamlException)actual).Start.Line.Should().Be(2);
((YamlException)actual).Start.Index.Should().Be(12);
((YamlException)actual).Start.Index.Should().Be(Environment.OSVersion.Platform == PlatformID.Unix ? 11 : 12);
((YamlException)actual).End.Column.Should().Be(4);
((YamlException)actual).End.Line.Should().Be(2);
((YamlException)actual).End.Index.Should().Be(15);
((YamlException)actual).End.Index.Should().Be(Environment.OSVersion.Platform == PlatformID.Unix ? 14 : 15);
((YamlException)actual).Message.Should().Be("Property 'bbb' not found on type 'YamlDotNet.Test.Serialization.Simple'.");
}

Expand Down Expand Up @@ -2142,7 +2142,7 @@ public void RoundtripWindowsNewlines()

using var reader = new StringReader(serialized);
var roundtrippedText = dut.Deserialize<StringContainer>(reader).Text.NormalizeNewLines();
Assert.Equal(text, roundtrippedText);
Assert.Equal(text, Environment.OSVersion.Platform == PlatformID.Win32NT ? roundtrippedText : roundtrippedText.Replace("\n", "\r\n"));
}

[TypeConverter(typeof(DoublyConvertedTypeConverter))]
Expand Down
102 changes: 51 additions & 51 deletions YamlDotNet.Test/Spec/ParserSpecTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,71 +39,71 @@ private sealed class ParserSpecTestsData : SpecTestsData

private static readonly List<string> ignoredSuites = new List<string>
{
// no spec test is ignored as of https://github.com/yaml/yaml-test-suite/releases/tag/data-2020-02-11
"5T43", "JR7V", "NKF9", "CFD4", "U99R"
Copy link
Contributor Author

@am11 am11 Aug 18, 2021

Choose a reason for hiding this comment

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

@aaubry, I have started working on fixing these new cases; got the first one fixed here am11@5943247.


Plain style scanning

From parsing architecture perspective, one significant issue is that we have FetchPlainScalar in Scanner class taking up a lot of responsibility and glues up a lot of mixed concerns (from all kinds of plain styles) in one place; which makes it harder to comprehend, if not impossible.

In particular, YAML 1.2 §7.3.3 talks about plain style of flow scalars, that is more restrictive than the $other plain styles of other scalars, and other styles of flow scalars. For the other two flow scalar styles (single and double quoted), we have a dedicated method FetchFlowScalar(bool isSingleQuoted), but for plain style the control flow either resorts to FetchValue, or (like in case of 5T43's fix) to FetchPlainScalar. IMHO, we should try to refactor flow scalar's plain style by changing its method's signature to FetchFlowScalar(bool isQuoted, bool isSingleQuoted) and implement its rules there. It might result in a little code duplication but much cleaner and implementation would be easily relatable with the spec, 1:1.

Similarly, for other kinds of plain styles, they could be structured in their corresponding parent concern and hence made relatable to the spec.


However, I believe priority 1 is correctness. Refactorings / optimizations can wait. I just wanted to record this message as the pain of finding the problem and my wounds therefore, are fresh at the moment. 8-)
Maybe someone has time to undergo this (very targetted) refactoring. My hands are full with stuff like real life, work and tracking down why JR7V and other are failing.. 🤣

};

private static readonly List<string> knownFalsePositives = new List<string>
{
// no false-positives known as of https://github.com/yaml/yaml-test-suite/releases/tag/data-2020-02-11
// no false-positives known as of https://github.com/yaml/yaml-test-suite/releases/tag/data-2020-08-01
};

private static readonly List<string> knownParserDesyncInErrorCases = new List<string>
{
"5LLU" // remove 5LLU once https://github.com/yaml/yaml-test-suite/pull/61 is released
// no desync cases known as of https://github.com/yaml/yaml-test-suite/releases/tag/data-2020-08-01
};

[Theory, ClassData(typeof(ParserSpecTestsData))]
public void ConformsWithYamlSpec(string name, string description, string inputFile, string expectedEventFile, bool error)
{
var expectedResult = File.ReadAllText(expectedEventFile);
using var writer = new StringWriter();
try
{
using var reader = File.OpenText(inputFile);
new LibYamlEventStream(new Parser(reader)).WriteTo(writer);
}
catch (Exception ex)
{
Assert.True(error, $"Unexpected spec failure ({name}).\n{description}\nExpected:\n{expectedResult}\nActual:\n[Writer Output]\n{writer}\n[Exception]\n{ex}");
if (error)
{
Debug.Assert(!knownFalsePositives.Contains(name), $"Spec test '{name}' passed but present in '{nameof(knownFalsePositives)}' list. Consider removing it from the list.");
try
{
Assert.Equal(expectedResult, writer.ToString(), ignoreLineEndingDifferences: true);
Debug.Assert(!knownParserDesyncInErrorCases.Contains(name), $"Spec test '{name}' passed but present in '{nameof(knownParserDesyncInErrorCases)}' list. Consider removing it from the list.");
}
catch (EqualException)
{
// In some error cases, YamlDotNet's parser output is in desync with what is expected by the spec.
// Throw, if it is not a known case.
if (!knownParserDesyncInErrorCases.Contains(name))
{
throw;
}
}
}
return;
}
try
{
Assert.Equal(expectedResult, writer.ToString(), ignoreLineEndingDifferences: true);
Debug.Assert(!ignoredSuites.Contains(name), $"Spec test '{name}' passed but present in '{nameof(ignoredSuites)}' list. Consider removing it from the list.");
}
catch (EqualException)
{
// In some cases, YamlDotNet's parser/scanner is unexpectedly *not* erroring out.
// Throw, if it is not a known case.
if (!(error && knownFalsePositives.Contains(name)))
{
throw;
}
using var writer = new StringWriter();
try
{
using var reader = File.OpenText(inputFile);
new LibYamlEventStream(new Parser(reader)).WriteTo(writer);
}
catch (Exception ex)
{
Assert.True(error, $"Unexpected spec failure ({name}).\n{description}\nExpected:\n{expectedResult}\nActual:\n[Writer Output]\n{writer}\n[Exception]\n{ex}");

if (error)
{
Debug.Assert(!knownFalsePositives.Contains(name), $"Spec test '{name}' passed but present in '{nameof(knownFalsePositives)}' list. Consider removing it from the list.");

try
{
Assert.Equal(expectedResult, writer.ToString(), ignoreLineEndingDifferences: true);
Debug.Assert(!knownParserDesyncInErrorCases.Contains(name), $"Spec test '{name}' passed but present in '{nameof(knownParserDesyncInErrorCases)}' list. Consider removing it from the list.");
}
catch (EqualException)
{
// In some error cases, YamlDotNet's parser output is in desync with what is expected by the spec.
// Throw, if it is not a known case.

if (!knownParserDesyncInErrorCases.Contains(name))
{
throw;
}
}
}

return;
}

try
{
Assert.Equal(expectedResult, writer.ToString(), ignoreLineEndingDifferences: true);
Debug.Assert(!ignoredSuites.Contains(name), $"Spec test '{name}' passed but present in '{nameof(ignoredSuites)}' list. Consider removing it from the list.");
}
catch (EqualException)
{
// In some cases, YamlDotNet's parser/scanner is unexpectedly *not* erroring out.
// Throw, if it is not a known case.

if (!(error && knownFalsePositives.Contains(name)))
{
throw;
}
}
}
}
Expand Down
71 changes: 36 additions & 35 deletions YamlDotNet.Test/Spec/SerializerSpecTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,13 @@ internal sealed class SerializerSpecTestsData : SpecTestsData
"KSS4", "KZN9", "L94M", "LE5A", "LP6E", "LQZ7", "M29M", "M5C3", "M7A3", "M7NX", "M9B4", "MJS9", "MYW6", "MZX3", "NAT4",
"NB6Z", "NHX8", "NJ66", "NP9H", "P2AD", "P76L", "PRH3", "PUW8", "PW8X", "Q88A", "Q8AD", "QT73", "R4YG", "R52L", "RTP8",
"RZP5", "RZT7", "S3PD", "S4JQ", "S4T7", "S7BG", "SKE5", "SSW6", "T4YY", "T5N4", "U3C3", "U3XV", "U9NS", "UGM3", "UT92",
"V55R", "W42U", "W4TN", "W5VH", "WZ62", "X38W", "X8DW", "XLQ9", "XV9V", "XW4D", "Y2GN", "Z67P", "Z9M4", "ZH7C", "ZWK4"
"V55R", "W42U", "W4TN", "W5VH", "WZ62", "X38W", "X8DW", "XLQ9", "XV9V", "XW4D", "Y2GN", "Z67P", "Z9M4", "ZH7C", "ZWK4",
"5T43", "NKF9", "CFD4", "JR7V"
};

private static readonly List<string> knownFalsePositives = new List<string>
{
// no false-positives known as of https://github.com/yaml/yaml-test-suite/releases/tag/data-2020-02-11
// no false-positives known as of https://github.com/yaml/yaml-test-suite/releases/tag/data-2020-08-01
};

private readonly IDeserializer deserializer = new DeserializerBuilder().Build();
Expand All @@ -64,39 +65,39 @@ internal sealed class SerializerSpecTestsData : SpecTestsData
public void ConformsWithYamlSpec(string name, string description, string inputFile, string outputFile, bool error)
{
var expectedResult = File.ReadAllText(outputFile);
using var writer = new StringWriter();
try
{
using var reader = File.OpenText(inputFile);
var subject = deserializer.Deserialize(reader);
serializer.Serialize(writer, subject);
}
catch (Exception ex)
{
Assert.True(error, $"Unexpected spec failure ({name}).\n{description}\nExpected:\n{expectedResult}\nActual:\n[Writer Output]\n{writer}\n[Exception]\n{ex}");
if (error)
{
Debug.Assert(!knownFalsePositives.Contains(name), $"Spec test '{name}' passed but present in '{nameof(knownFalsePositives)}' list. Consider removing it from the list.");
}
return;
}
try
{
Assert.Equal(expectedResult, writer.ToString(), ignoreLineEndingDifferences: true);
Debug.Assert(!ignoredSuites.Contains(name), $"Spec test '{name}' passed but present in '{nameof(ignoredSuites)}' list. Consider removing it from the list.");
}
catch (EqualException)
{
// In some cases, YamlDotNet's parser/scanner is unexpectedly *not* erroring out.
// Throw, if it is not a known case.
if (!(error && knownFalsePositives.Contains(name)))
{
throw;
}
using var writer = new StringWriter();
try
{
using var reader = File.OpenText(inputFile);
var subject = deserializer.Deserialize(reader);
serializer.Serialize(writer, subject);
}
catch (Exception ex)
{
Assert.True(error, $"Unexpected spec failure ({name}).\n{description}\nExpected:\n{expectedResult}\nActual:\n[Writer Output]\n{writer}\n[Exception]\n{ex}");

if (error)
{
Debug.Assert(!knownFalsePositives.Contains(name), $"Spec test '{name}' passed but present in '{nameof(knownFalsePositives)}' list. Consider removing it from the list.");
}

return;
}

try
{
Assert.Equal(expectedResult, writer.ToString(), ignoreLineEndingDifferences: true);
Debug.Assert(!ignoredSuites.Contains(name), $"Spec test '{name}' passed but present in '{nameof(ignoredSuites)}' list. Consider removing it from the list.");
}
catch (EqualException)
{
// In some cases, YamlDotNet's parser/scanner is unexpectedly *not* erroring out.
// Throw, if it is not a known case.

if (!(error && knownFalsePositives.Contains(name)))
{
throw;
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion YamlDotNet.Test/yaml-test-suite
Submodule yaml-test-suite updated 304 files