Pull Request Test Coverage Report for Build 4643984026

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 45 of 45 (100.0%) changed or added relevant lines in 6 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.01%) to 97.2%

Totals
Change from base Build 4629906477:	0.01%
Covered Lines:	12881
Relevant Lines:	13124

💛 - Coveralls

Hi @dennisdoomen @lg2de, is there anything else to do for this PR?

Hi @dennisdoomen @lg2de, is there anything else to do for this PR?

No, waiting for @jnyrup to find some time to review the PR as well.

@dennisdoomen Will you have another look since the implementation changed some since your approval.

❓Wouldn't it make the output better readable if we add an empty line after a multi-line "Expected..." statement? Just like we do in C#.

I'm not entirely sure what you mean here @dennisdoomen, is this the same as #2144 (comment)? If not, could you elaborate a little please?

@benagain sorry for some of the new comments. But as @jnyrup requested, I had a bit of a more thorough look at the PR (and the output we used to generate). Very happy with the improvements. We just need to make sure it's picture perfect.

No problem, I'm quite enjoying polishing it to get it right.

I'm not entirely sure what you mean here @dennisdoomen, is this the same as #2144 (comment)? If not, could you elaborate a little please?

Yeah, I meant the same thing there.

@benagain I assume you're still working on this PR. Ping me when you're done and I can conclude the review.

@benagain I assume you're still working on this PR. Ping me when you're done and I can conclude the review.

Hi @dennisdoomen, I found an annoying problem that has been tricky to resolve. As it currently stands a list of objects will be formatted with the list's opening brace and the first object's name on the same line as "Expected...", rather than on a new line:

  Expected stuff to be equal to {FluentAssertions.Specs.Formatting.FormatterSpecs+Stuff`1[[System.Int32*]]
      {
          Children = {1, 2, 3, 4}, 
  ...

This comes down to the interplay between EnumerableValueFormatter and FormattedObjectGraph and is challenging because simple, short lists should be formatted on the original line:

Expected list to be equal to {1,2,3,4}

I'm fairly close to a solution but I will be the first to say it's a touch convoluted. Maybe when I show the code someone will have a better idea.

@dennisdoomen, @lg2de, @jnyrup, and anyone else interested!..

I'd like to draw your attention to the test When_the_object_is_a_generic_type_without_custom_string_representation_it_should_show_the_properties:

I think this formatting is what we're after, especially the newline after "expected stuff to be", and indentation of the type name to match the type contents.

This particular stuff is a list of complex objects, and the EnumerableFormatter does not know whether or not to put that opening brace on a newline until it has formatted at least one child. Lists of integers should be formatted on a single line:

Expected numbers to be {1, 2, 3, 4}

I tried a few different things, then came up with PossibleMultilineFragment formatter which tracks where the brace should go in the output, and then insert it later on after we know if this enumerable is single-line or spans multiple lines. EnumerableFormatter was already making a "single or multi-line" decision so I think this follow that pattern. Inserting content into the graph post-hoc required more access to the internals of FormattedObjectGraph so I felt it should be a responsibility of that now.

Please have a look. It seems a bit overly complicated for what should be something simple but I have not yet thought of a better solution whilst keeping the desired formatting.

I like what I see in the new tests!