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

fix: Fix duplicate invocation of deprecated onTestFinish callback #1317

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

christophd
Copy link
Member

Does it make sense to invoke the deprecated onTestFinish() in new onTestExecutionStart()? I guess onTestFinish method would be called twice as the new onTestExecutionEnd() does also call the deprecated method, too.

@christophd christophd requested a review from tschlat February 14, 2025 08:51
@@ -42,7 +42,7 @@ default void onTestFinish(TestCase test) {
* @see #onTestEnd(TestCase)
*/
default void onTestExecutionStart(TestCase test) {
onTestFinish(test);
// Default implementation does nothing
Copy link
Collaborator

Choose a reason for hiding this comment

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

😵

This was a last minute change using copy paste. The correct documentation and implementation should be:
(sorry for not being able to suggest on multiple lines - do not apply as patch)

Suggested change
// Default implementation does nothing
/**
* Invoked when test execution starts (after {@link org.citrusframework.container.BeforeTest} execution)
*/
default void onTestExecutionStart(TestCase test) {
// Default implementation does nothing
}

Copy link
Member Author

Choose a reason for hiding this comment

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

thank you, I have fixed the javadocs, too. as you have suggested

@christophd christophd force-pushed the chore/test-listener-on-finish branch from 36d9b4e to 6399728 Compare February 14, 2025 18:21
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.

2 participants