Skip to content

Hook extension improvements#2

Merged
z002Holpp merged 2 commits intoH-Devfrom
HookExtensionImprovements
Apr 15, 2025
Merged

Hook extension improvements#2
z002Holpp merged 2 commits intoH-Devfrom
HookExtensionImprovements

Conversation

@z002Holpp
Copy link
Collaborator

Simplification of Hook Extensions.
Removed unnecessary async handlers.
Fixed HookExtension copy constructor and added tests.

…s are propagates correctly from the handlers to NUnit. Async handlers are still executed asynchronously inside the AsyncEvent.
Copy link
Owner

@TSAVogt TSAVogt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!

var exception = new Exception("Test exception");
var testMethodEventArgs = new TestHookTestMethodEventArgs(null);
asyncEvent.AddAsyncHandler(async (sender, args) => throw exception);
Assert.Throws<AggregateException>(() => invoke(this, testMethodEventArgs).Wait());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wondering about this wait? Didn't I remove that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this wait is in principle correct here, however it would not be needed since the exception is thrown

var exception = new Exception("Test exception");
var testMethodEventArgs = new TestHookTestMethodEventArgs(null);
syncEvent.AddHandler((sender, args) => throw exception);
Assert.Throws<AggregateException>(() => invoke(this, testMethodEventArgs).Wait());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait?

@z002Holpp z002Holpp merged commit d56e6e6 into H-Dev Apr 15, 2025
0 of 3 checks passed
@z002Holpp z002Holpp deleted the HookExtensionImprovements branch April 15, 2025 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants