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

AssertionScope with a custom strategy does not report each assertion #1118

Closed
tomaustin700 opened this issue Aug 19, 2019 · 5 comments
Closed

AssertionScope with a custom strategy does not report each assertion #1118

tomaustin700 opened this issue Aug 19, 2019 · 5 comments
Assignees
Labels

Comments

@tomaustin700
Copy link
Contributor

@tomaustin700 tomaustin700 commented Aug 19, 2019

Description

Hi,

I am trying to use the new custom strategy logic (introduced on PR1904) so I can take a screenshot on test failure however when I have implemented this it is only asserting on the first assert and not reporting any additional asserts.

Complete minimal example reproducing the issue

Here is a custom strategy which is just a copy of CollectingAssertionStrategy to demonstrate the issue.

  public class CustomAssertionStrategy : IAssertionStrategy
    {
        private readonly List<string> failureMessages = new List<string>();

        /// <summary>
        /// Returns the messages for the assertion failures that happened until now.
        /// </summary>
        public IEnumerable<string> FailureMessages => failureMessages;

        /// <summary>
        /// Discards and returns the failure messages that happened up to now.
        /// </summary>
        public IEnumerable<string> DiscardFailures()
        {
            var discardedFailures = failureMessages.ToArray();
            failureMessages.Clear();
            return discardedFailures;
        }

        /// <summary>
        /// Will throw a combined exception for any failures have been collected since <see cref="StartCollecting"/> was called.
        /// </summary>
        public void ThrowIfAny(IDictionary<string, object> context)
        {
            if (failureMessages.Any())
            {
                var builder = new StringBuilder();
                builder.AppendLine(string.Join(Environment.NewLine, failureMessages));

                if (context.Any())
                {
                    foreach (KeyValuePair<string, object> pair in context)
                    {
                        builder.AppendFormat("\nWith {0}:\n{1}", pair.Key, pair.Value);
                    }
                }

                Services.ThrowException(builder.ToString());
            }
        }

        /// <summary>
        /// Instructs the strategy to handle a assertion failure.
        /// </summary>
        public void HandleFailure(string message)
        {
            failureMessages.Add(message);
        }
    }

and then

using (var scope = new AssertionScope(new CustomAssertionStrategy()))
            {

                false.Should().BeTrue();
                true.Should().BeFalse();
            }

Expected behavior:

Both assertion failures should be reported when using a custom strategy

Actual behavior:

Only the first assertion failure is reported

Versions

5.8.0, .Net Framework 4.7.1

Additional Information

See https://github.com/tomaustin700/FluentAssertionIssue for a full solution demonstrating this issue.

@jnyrup
Copy link
Member

@jnyrup jnyrup commented Aug 20, 2019

Hi Tom
Thanks for the descriptive bug report and example.
That made the identification and fixing (close to) a no-brainer.

Loading

@tomaustin700
Copy link
Contributor Author

@tomaustin700 tomaustin700 commented Aug 20, 2019

@jnyrup No problem, thanks for the quick fix! 👍

Loading

@ThomasPoucher
Copy link

@ThomasPoucher ThomasPoucher commented Aug 28, 2019

@jnyrup Any update on when this fix will be released?

Loading

@eNeRGy164
Copy link
Contributor

@eNeRGy164 eNeRGy164 commented Aug 28, 2019

i think 8 minutes ago :P

Loading

@dennisdoomen
Copy link
Member

@dennisdoomen dennisdoomen commented Aug 28, 2019

Yep. Just released it.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants