Ensures expectations involving multiple assertions can be cleared #925
Conversation
e99ff63 to
37cc1bb
Compare
37cc1bb to
c54c008
Compare
jnyrup
left a comment
There was a problem hiding this comment.
I'm not sure what the functional consequences are?
| /// Indicates that every argument passed into <see cref="FailWith"/> is displayed on a separate line. | ||
| /// </summary> | ||
| public AssertionScope UsingLineBreaks | ||
| public IAssertionScope UsingLineBreaks |
There was a problem hiding this comment.
🤔 As IAssertionScope doesn't have all the public members of AssertionScope this is a breaking change
There was a problem hiding this comment.
I'm not sure. The members of IAssertionScope are only used in chaining scenarios. I've included a couple of them, but not all. Any strong opinions?
There was a problem hiding this comment.
I haven't tried to invoke AssertionScope-only members myself, I just looked for changes that could break the public API.
There was a problem hiding this comment.
Normally, you're directly using the AssertionScope in a using block. The interface is only used for those complex chaining purposes. In the last commit, I've added almost all of them to the interface, just to be sure.
There was a problem hiding this comment.
But no, I don't have any strong opinions, except trying to avoid breaking changes.
Just as a note: the remaining differences between AssertionScope and IAssertionScope are:
public string Context { get; set; }
public void AddPreFormattedFailure(string formattedFailureMessage);
void AddNonReportable(string key, object value);
void AddReportable(string key, string value);
T Get<T>(string key);There was a problem hiding this comment.
Yes, I know. These are very specific things, mostly only used by BeEquivalentTo.
c54c008 to
9356b83
Compare
jnyrup
left a comment
There was a problem hiding this comment.
Would these changes benefit from any tests exercising the code?
|
I'm in doubt. I would have to check whether the failure doesn't contain anything else. Not sure if it's worth though. I'll check when I continue with #911. |
Introduces a
ClearExpectationto undo the expectation that is associated with multiple assertions usingWithExpectation. Without it, it is possible that this expectation gets appended to any successive assertions.Fixes #918