Allow event monitoring to ignore failing event accessors #1954

apazureck · 2022-07-11T18:09:25Z

Added options to event monitor and added option to suprress exceptions on event accessors.

Closes #1948

IMPORTANT

The code complies with the Coding Guidelines for C#.
The changes are covered by unit tests which follow the Arrange-Act-Assert syntax and the naming conventions such as is used in these tests.
If the PR adds a feature or fixes a bug, please update the release notes with a functional description that explains what the change means to consumers of this library, which are published on the website.
If the PR changes the public API the changes needs to be included by running AcceptApiChanges.ps1 or AcceptApiChanges.sh.
If the PR affects the documentation, please include your changes in this pull request so the documentation will appear on the website.

Moved methods config options to event monitor options

# Conflicts: # docs/_pages/releases.md

coveralls · 2022-07-12T16:02:02Z

Pull Request Test Coverage Report for Build 2823614045

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

30 of 30 (100.0%) changed or added relevant lines in 3 files are covered.
5 unchanged lines in 2 files lost coverage.
Overall coverage increased (+1.0%) to 96.643%

Files with Coverage Reduction	New Missed Lines	%
Src/FluentAssertions/AssertionExtensions.cs	1	95.68%
Src/FluentAssertions/Events/EventMonitor.cs	4	92.78%

Totals
Change from base Build 2822953981:	1.0%
Covered Lines:	12331
Relevant Lines:	12599

💛 - Coveralls

Src/FluentAssertions/Events/EventMonitorOptions.cs

Tests/FluentAssertions.Specs/FluentAssertions.Specs.csproj

docs/_pages/eventmonitoring.md

docs/_pages/releases.md

Src/FluentAssertions/Events/EventMonitorOptions.cs

Tests/FluentAssertions.Specs/Events/EventMonitorSpecs.cs

Fixed wrong Timestamp

Removed unused moq

… into develop

jnyrup · 2022-07-13T09:04:08Z

Btw, I'm AFK for a week from today. I'll continue reviewing once I'm back.

apazureck · 2022-07-14T07:17:37Z

Hi,

Quick question: I did changes on my fork and pushed it. Sadly github does not seem to detect them in this PR. Do you know how I can update the PR?

dennisdoomen · 2022-07-14T08:01:37Z

Are these not your changes?

apazureck · 2022-07-15T06:51:20Z

Ah ok... odd. This did not show up, that's why I was a bit confused (hadn't used github in some time...)

Src/FluentAssertions/Events/EventMonitor.cs

Src/FluentAssertions/AssertionExtensions.cs

Src/FluentAssertions/Events/EventMonitor.cs

Tests/FluentAssertions.Specs/Events/EventAssertionSpecs.cs

Co-authored-by: Jonas Nyrup <jnyrup@users.noreply.github.com>

Src/FluentAssertions/Events/EventMonitor.cs

docs/_pages/releases.md

docs/_pages/eventmonitoring.md

IT-VBFK · 2023-07-14T19:57:11Z

And API-checks..

Tests/FluentAssertions.Specs/Events/EventAssertionSpecs.cs

dennisdoomen · 2023-08-19T18:29:35Z

@apazureck any change you will finish this PR?

IT-VBFK · 2023-08-23T09:36:13Z

Would this go into 6.12 if someone finishes it?

apazureck · 2023-08-23T09:38:26Z

Hi,

I will fix the stuff until Friday. Thought it was OK and already finished.

# Conflicts: # docs/_pages/releases.md

github-actions · 2023-08-25T19:43:09Z

Qodana for .NET

It seems all right 👌

No new problems were found according to the checks applied

💡 Qodana analysis was run in the pull request mode: only the changed files were checked
☁️ View the detailed Qodana report

Contact Qodana team

Contact us at qodana-support@jetbrains.com

Or via our issue tracker: https://jb.gg/qodana-issue
Or share your feedback: https://jb.gg/qodana-discussions

Co-authored-by: IT-VBFK <49762557+IT-VBFK@users.noreply.github.com>

IT-VBFK · 2023-08-26T05:42:40Z

Can you also take care of the remaining qodana issues?

apazureck · 2023-08-26T06:06:21Z

Can you also take care of the remaining qodana issues?

Hi,

Yes, I am on it. But the job took so long to run I went to bed yesterday.

…broken accessor

Updated descriptions to match current the actual behavior

jnyrup · 2023-08-27T14:53:41Z

Tests/FluentAssertions.Specs/Events/EventAssertionSpecs.cs

+        public event EventHandler RemoveFailingEvent;
+    }
+
+    [SuppressMessage("Usage", "CA1801:Check Unused Parameter", Justification = "This is on purpose for testing.")]


What parameter is unused?
I didn't get a warning when removing this.

jnyrup · 2023-08-27T14:53:58Z

docs/_pages/releases.md

@@ -7,31 +7,13 @@ sidebar:
  nav: "sidebar"
 ---

-## 7.0 Alpha 1


Something went wrong here 🙃

jnyrup · 2023-08-27T15:07:05Z

Src/FluentAssertions/AssertionExtensions.cs

    public static IMonitor<T> Monitor<T>(this T eventSource, Func<DateTime> utcNow = null)
    {
-        return new EventMonitor<T>(eventSource, utcNow ?? (() => DateTime.UtcNow));
+        var options = new EventMonitorOptions();
+
+        if (utcNow is not null)
+        {
+            options.ConfigureTimestampProvider(utcNow);
+        }
+
+        return new EventMonitor<T>(eventSource, options);
+    }


Now that we're open to change the existing API, we could remove Func<DateTime> utcNow = null, such that this becomes.

public static IMonitor<T> Monitor<T>(this T eventSource) => new EventMonitor<T>(eventSource, new EventMonitorOptions());

For testing we still have the internal ConfigureTimestampProvider.

@dennisdoomen What do you think?

For testing we still have the internal ConfigureTimestampProvider.

What's that?

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.

Ah yes, you're right. That would no longer be necessary.

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.

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.

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.

In #625 the Func utcNow = null was added, but I suspect it wasn't meant for public use, but merely a necessity for testing.

We should mention this in the docs as well :)

Co-authored-by: IT-VBFK <49762557+IT-VBFK@users.noreply.github.com>

dennisdoomen · 2023-09-14T17:16:24Z

You'll need to rebase your changes on develop to resolve the conflicts.

IT-VBFK · 2023-10-27T08:37:17Z

Ping

apazureck · 2023-11-06T08:47:00Z

Thanks for the reminder, I thought it was already finished. I can tend to this on the upcoming weekend and hope it will pass then.

ITaluone · 2023-11-20T06:21:17Z

Pong

ITaluone · 2024-04-22T09:42:27Z

@apazureck Do you have a plan on finishing this?

Removed event monitor builder

3c3afc9

Moved methods config options to event monitor options

apazureck changed the title ~~Draft Removed event monitor builder~~ Draft: Removed event monitor builder Jul 11, 2022

Restructured and extended the event monitoring documentation

3272e9d

apazureck changed the title ~~Draft: Removed event monitor builder~~ [WIP] Event Monitor Options Jul 12, 2022

updated release notes

d1f5a6e

apazureck changed the title ~~[WIP] Event Monitor Options~~ Event Monitor Options Jul 12, 2022

Andreas Pazureck added 4 commits July 12, 2022 10:35

Merge remote-tracking branch 'root/develop' into develop

5cec0a5

# Conflicts: # docs/_pages/releases.md

Removed unnecessary generic type from EventMonitorOptions

9388b89

fixed build errors due to removing generic from options

d66cc9c

Update api checks

e783b98

jnyrup reviewed Jul 12, 2022

View reviewed changes

Src/FluentAssertions/Events/EventMonitorOptions.cs Outdated Show resolved Hide resolved

Tests/FluentAssertions.Specs/FluentAssertions.Specs.csproj Outdated Show resolved Hide resolved

dennisdoomen requested changes Jul 12, 2022

View reviewed changes

apazureck and others added 4 commits July 13, 2022 10:45

Update Src/FluentAssertions/Events/EventMonitorOptions.cs

2a5d159

Fixed wrong Timestamp

Update Tests/FluentAssertions.Specs/FluentAssertions.Specs.csproj

4cdf1e2

Removed unused moq

fixed wrong documentation

2bab3e1

Merge branch 'develop' of https://github.com/apazureck/fluentassertions…

d1d149c

… into develop

included reviewer feedback

9e185d7

jnyrup reviewed Jul 21, 2022

View reviewed changes

dennisdoomen approved these changes Jul 22, 2022

View reviewed changes

jnyrup added the feature label Jul 24, 2022

apazureck and others added 4 commits August 5, 2022 13:36

Update Tests/FluentAssertions.Specs/Events/EventAssertionSpecs.cs

bf5702c

Co-authored-by: Jonas Nyrup <jnyrup@users.noreply.github.com>

Added more reviewer suggestions.

35cb61b

Merge remote-tracking branch 'origin/develop' into develop

7e07ae4

Merge remote-tracking branch 'root/develop' into develop

3bd2e5f

IT-VBFK reviewed Aug 5, 2022

View reviewed changes

Src/FluentAssertions/Events/EventMonitor.cs Outdated Show resolved Hide resolved

dennisdoomen approved these changes Jul 11, 2023

View reviewed changes

docs/_pages/releases.md Outdated Show resolved Hide resolved

dennisdoomen requested a review from jnyrup July 11, 2023 14:06

jnyrup added 2 commits July 14, 2023 21:40

cleanups

19209ff

fixup docs

6f6e555

IT-VBFK reviewed Jul 14, 2023

View reviewed changes

docs/_pages/eventmonitoring.md Outdated Show resolved Hide resolved

docs/_pages/eventmonitoring.md Outdated Show resolved Hide resolved

jnyrup reviewed Jul 14, 2023

View reviewed changes

Tests/FluentAssertions.Specs/Events/EventAssertionSpecs.cs Outdated Show resolved Hide resolved

apazureck added 2 commits August 25, 2023 21:28

Merge remote-tracking branch 'FARepo/develop' into develop

729a048

# Conflicts: # docs/_pages/releases.md

Merge remote-tracking branch 'origin/develop' into develop

e623306

# Conflicts: # docs/_pages/releases.md

apazureck and others added 2 commits August 25, 2023 21:52

Fixed Qodana complaints

d9bd625

Update docs/_pages/eventmonitoring.md

09b35f9

Co-authored-by: IT-VBFK <49762557+IT-VBFK@users.noreply.github.com>

apazureck requested a review from jnyrup August 25, 2023 19:54

apazureck added 4 commits August 26, 2023 08:11

Fixed more Qodana issues

db55f59

Merge remote-tracking branch 'origin/develop' into develop

6759a10

Removed boolean parameter and added method for recording events with …

d894f0f

…broken accessor

updated api tests for new RecordEventsWithBrokenAccessor method.

1b0eaaf

Updated descriptions to match current the actual behavior

jnyrup reviewed Aug 27, 2023

View reviewed changes

Update docs/_pages/eventmonitoring.md

0849e64

Co-authored-by: IT-VBFK <49762557+IT-VBFK@users.noreply.github.com>

ITaluone mentioned this pull request Apr 22, 2024

Allow event monitoring to ignore failing event accessors #2629

Open

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow event monitoring to ignore failing event accessors #1954

Allow event monitoring to ignore failing event accessors #1954

apazureck commented Jul 11, 2022 •

edited

coveralls commented Jul 12, 2022 •

edited

jnyrup commented Jul 13, 2022

apazureck commented Jul 14, 2022

dennisdoomen commented Jul 14, 2022

apazureck commented Jul 15, 2022

IT-VBFK commented Jul 14, 2023

dennisdoomen commented Aug 19, 2023

IT-VBFK commented Aug 23, 2023

apazureck commented Aug 23, 2023

github-actions bot commented Aug 25, 2023 •

edited

IT-VBFK commented Aug 26, 2023

apazureck commented Aug 26, 2023

jnyrup Aug 27, 2023

jnyrup Aug 27, 2023

jnyrup Aug 27, 2023

dennisdoomen Aug 27, 2023

jnyrup Aug 27, 2023

dennisdoomen Aug 27, 2023

apazureck Aug 28, 2023 •

edited

dennisdoomen Aug 28, 2023

apazureck Aug 28, 2023

ITaluone Sep 4, 2023

dennisdoomen commented Sep 14, 2023

IT-VBFK commented Oct 27, 2023

apazureck commented Nov 6, 2023

ITaluone commented Nov 20, 2023

ITaluone commented Apr 22, 2024

Allow event monitoring to ignore failing event accessors #1954

Are you sure you want to change the base?

Allow event monitoring to ignore failing event accessors #1954

Conversation

apazureck commented Jul 11, 2022 • edited

IMPORTANT

coveralls commented Jul 12, 2022 • edited

Pull Request Test Coverage Report for Build 2823614045

Warning: This coverage report may be inaccurate.

💛 - Coveralls

jnyrup commented Jul 13, 2022

apazureck commented Jul 14, 2022

dennisdoomen commented Jul 14, 2022

apazureck commented Jul 15, 2022

IT-VBFK commented Jul 14, 2023

dennisdoomen commented Aug 19, 2023

IT-VBFK commented Aug 23, 2023

apazureck commented Aug 23, 2023

github-actions bot commented Aug 25, 2023 • edited

Qodana for .NET

IT-VBFK commented Aug 26, 2023

apazureck commented Aug 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apazureck Aug 28, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dennisdoomen commented Sep 14, 2023

IT-VBFK commented Oct 27, 2023

apazureck commented Nov 6, 2023

ITaluone commented Nov 20, 2023

ITaluone commented Apr 22, 2024

apazureck commented Jul 11, 2022 •

edited

coveralls commented Jul 12, 2022 •

edited

github-actions bot commented Aug 25, 2023 •

edited

apazureck Aug 28, 2023 •

edited