dotnet / CliCommandLineParser

A second-generation parser for the .NET Core command line tools
MIT License
111 stars 44 forks source link

ParsingValidationTests failure in prodcon #93

Open tannergooding opened 6 years ago

tannergooding commented 6 years ago

The When_no_option_accepts_arguments_but_one_is_supplied_then_an_error_is_returned parsing validation test is failing in prodcon with:

<test name="Microsoft.DotNet.Cli.CommandLine.Tests.ParsingValidationTests.When_no_option_accepts_arguments_but_one_is_supplied_then_an_error_is_returned" type="Microsoft.DotNet.Cli.CommandLine.Tests.ParsingValidationTests" method="When_no_option_accepts_arguments_but_one_is_supplied_then_an_error_is_returned" time="0.0448245" result="Fail">
  <failure exception-type="Xunit.Sdk.XunitException">
    <message><![CDATA[Expected collection to contain a single item matching (e == \"Unrecognized command or argument 'some-arg'\"), but no such item was found.]]></message>
    <stack-trace><![CDATA[   at FluentAssertions.Execution.XUnit2TestFramework.Throw(String message) in C:\projects\fluentassertions-vf06b\Src\FluentAssertions.Net45\Execution\XUnit2TestFramework.cs:line 32
                             at FluentAssertions.Execution.AssertionScope.FailWith(String message, Object[] args) in C:\projects\fluentassertions-vf06b\Src\Core\Execution\AssertionScope.cs:line 197
                             at FluentAssertions.Collections.SelfReferencingCollectionAssertions`2.ContainSingle(Expression`1 predicate, String because, Object[] becauseArgs) in C:\projects\fluentassertions-vf06b\Src\Core\Collections\SelfReferencingCollectionAssertions.cs:line 492]]></stack-trace>
  </failure>
</test>

The test is not failing locally in any of the configurations I tested.

wli3 commented 6 years ago

@jonsequitur any clue? It should not be possible... everything is in memory

nguerrera commented 6 years ago

When this runs in prodcon is it using the corefx/coreclr that was built before it?

wli3 commented 6 years ago

Another theory is the code uses == for assertion. But it is object equal. String interning could cause this problem.

https://github.com/dotnet/CliCommandLineParser/blob/1bae8000e6f41eb4fb2c888ecb976b93ce1ba5af/src/tests/CommandLine.Tests/ParsingValidationTests.cs#L62-L72

nguerrera commented 6 years ago

No, string overloads operator ==(a, b) to be the same as String.Equals(a, b)

wli3 commented 6 years ago

true, without any extra stuff and the type of e is string.

https://github.com/dotnet/coreclr/blob/38cf93013c1dd1efc7137a6f4930cab7cc653411/src/mscorlib/src/System/String.Comparison.cs#L969

        public static bool operator ==(String a, String b)
        {
            return String.Equals(a, b);
        }
jonsequitur commented 6 years ago

ContainSingle accepts an expression, not a delegate. Depending on how FluentAssertions uses that expression, the result could be a call to object.Equals, which, if the string is not interned, can result in a false, e.g.:

object obj = "String";
string str = typeof(string).Name;
Console.WriteLine($"{obj} == {str}?"); // --> String == String?
Console.WriteLine(obj == str); // --> False

The question is why this would vary based on the build.

We're guessing that changing the comparison from == to .Equals might fix the problem.

nguerrera commented 6 years ago

Interesting take. Treating that expression as object equals would be a bug in FluentAssertions. Seems like we should dig in, verify that, and report it to them if we can demonstrate that's indeed what's happening.

nguerrera commented 6 years ago

It looks to me like FluentAssertions uses Expression<T>.Compile() so if there's a bug in the interpretation it would be there.

@tannergooding Repeating my question: Does prodcon execution of these tests use a just-built coreclr/corefx?

tannergooding commented 6 years ago

@mmitche might be a better person to ask.

Does prodcon execution of these tests use a just-built coreclr/corefx?

mmitche commented 6 years ago

@tannergooding No

nguerrera commented 6 years ago

@mmitche No, it doesn't get a just-built copy, or no you're not the better person to ask? ;)

mmitche commented 6 years ago

@nguerrera It doesn't use a just-build coreclr/corefx