-
-
Notifications
You must be signed in to change notification settings - Fork 540
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
Allow event monitoring to ignore failing event accessors #1954
Closed
Closed
Changes from 25 commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
3c3afc9
Removed event monitor builder
3272e9d
Restructured and extended the event monitoring documentation
d1f5a6e
updated release notes
5cec0a5
Merge remote-tracking branch 'root/develop' into develop
9388b89
Removed unnecessary generic type from EventMonitorOptions
d66cc9c
fixed build errors due to removing generic from options
e783b98
Update api checks
2a5d159
Update Src/FluentAssertions/Events/EventMonitorOptions.cs
apazureck 4cdf1e2
Update Tests/FluentAssertions.Specs/FluentAssertions.Specs.csproj
apazureck 2bab3e1
fixed wrong documentation
d1d149c
Merge branch 'develop' of https://github.com/apazureck/fluentassertio…
9e185d7
included reviewer feedback
bf5702c
Update Tests/FluentAssertions.Specs/Events/EventAssertionSpecs.cs
apazureck 35cb61b
Added more reviewer suggestions.
7e07ae4
Merge remote-tracking branch 'origin/develop' into develop
3bd2e5f
Merge remote-tracking branch 'root/develop' into develop
34810c5
Update Src/FluentAssertions/Events/EventMonitor.cs
apazureck 08fc020
Update Src/FluentAssertions/Events/EventMonitorOptions.cs
apazureck 61ec8a8
Merge branch 'develop' into develop
apazureck c58bb5e
Merge remote-tracking branch 'FARepo/develop' into develop
apazureck a1fad9b
fixed erros due to changes in the main repo
apazureck e4fdc3a
Merge remote-tracking branch 'FARepo/develop' into develop
apazureck 9c8bbb5
- removed the exception when event recorder could not be removed from…
apazureck 19209ff
cleanups
jnyrup 6f6e555
fixup docs
jnyrup 729a048
Merge remote-tracking branch 'FARepo/develop' into develop
apazureck e623306
Merge remote-tracking branch 'origin/develop' into develop
apazureck d9bd625
Fixed Qodana complaints
apazureck 09b35f9
Update docs/_pages/eventmonitoring.md
apazureck db55f59
Fixed more Qodana issues
apazureck 6759a10
Merge remote-tracking branch 'origin/develop' into develop
apazureck d894f0f
Removed boolean parameter and added method for recording events with …
apazureck 1b0eaaf
updated api tests for new RecordEventsWithBrokenAccessor method.
apazureck 0849e64
Update docs/_pages/eventmonitoring.md
apazureck File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
using System; | ||
|
||
namespace FluentAssertions.Events; | ||
|
||
/// <summary> | ||
/// Settings for the <see cref="EventMonitor{T}"/>. | ||
/// </summary> | ||
public class EventMonitorOptions | ||
{ | ||
/// <summary> | ||
/// Will ignore the events, if they throw an exception on any custom event accessor implementation. default: false. | ||
/// </summary> | ||
internal bool ShouldIgnoreEventAccessorExceptions { get; private set; } | ||
|
||
/// <summary> | ||
/// This will record the event, even if the event accessor add event accessor threw an exception. To ignore exceptions in the event add accessor, call <see cref="IgnoreEventAccessorExceptions"/> property to true. default: false. | ||
/// </summary> | ||
internal bool RecordEventsWithBrokenAccessor { get; private set; } | ||
|
||
/// <summary> | ||
/// Func used to generate the timestamp. | ||
/// </summary> | ||
internal Func<DateTime> TimestampProvider { get; private set; } = () => DateTime.UtcNow; | ||
|
||
/// <summary> | ||
/// When called it will ignore event accessor Exceptions. | ||
/// </summary> | ||
/// <param name="recordEventsWithBrokenAccessor">This will record the event, even if the event add event accessor threw an exception. default: false.</param> | ||
/// <returns>The options instance for method stacking.</returns> | ||
public EventMonitorOptions IgnoreEventAccessorExceptions(bool recordEventsWithBrokenAccessor = false) | ||
{ | ||
ShouldIgnoreEventAccessorExceptions = true; | ||
RecordEventsWithBrokenAccessor = recordEventsWithBrokenAccessor; | ||
return this; | ||
} | ||
|
||
/// <summary> | ||
/// Sets the timestamp provider. By default it is <see cref="DateTime.UtcNow"/>. | ||
/// </summary> | ||
/// <param name="timestampProvider">The timestamp provider.</param> | ||
/// <returns>The options instance for method stacking.</returns> | ||
internal EventMonitorOptions ConfigureTimestampProvider(Func<DateTime> timestampProvider) | ||
{ | ||
if (timestampProvider != null) | ||
{ | ||
TimestampProvider = timestampProvider; | ||
} | ||
|
||
return this; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that we're open to change the existing API, we could remove
Func<DateTime> utcNow = null
, such that this becomes.For testing we still have the internal
ConfigureTimestampProvider
.@dennisdoomen What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EventMonitorOptions.ConfigureTimestampProvider
.In #625 the
Func<DateTime> utcNow = null
was added, but I suspect it wasn't meant for public use, but merely a necessity for testing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, you're right. That would no longer be necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have a workflow for obsolete methods? Should it be tagged obsolete first with the remark it will be removed in the next version? Then people have some time, if they are using it for some reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's how we normally do it. But last week, the
develop
branch is used for the next major version, so we're fine to introduce well-argumented breaking changes in that branch.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the utcnow. I handed down the options to the eventrecorder, as otherwise the late changed reference on the timestamp provider would not be used. The events are subscribed when the constructor is called. Please check if it is conform to your other code and let me know, if it you like another approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should mention this in the docs as well :)