feat(#1218): add events on TestListener

[phos-web]

Related issue.

The following methods have been added to the TestListener interface:

default void onTestEnd(TestCase test)
default void onFinalActionsEnd(TestCase test)
default void onAfterSequenceBeforeTest(TestCase test)
default void onAfterSequenceAfterTest(TestCase test)
default void onBeforeSequenceBeforeTest(TestCase test)
default void onBeforeSequenceAfterTest(TestCase test)

Tests have been written in DefaultCaseTest

[christophd]

what is the difference of onTestEnd compared to the existing onTestFinished?

[christophd]

Maybe we can rename onAfterSequenceBeforeTest to onBeforeTestFinished and onAfterSequenceAfterTest to onAfterTestFinished

[christophd]

Some comments to better understand the use case. Many thanks!

[tschlat]

Some comments to better understand the use case. Many thanks!

Could you please check the associated issued #1218? I added an example there. Hope the use case gets clear from that.

[tschlat]

what is the difference of onTestEnd compared to the existing onTestFinished?

Please re-check the javadoc. Hope that we have documented it well now.

[tschlat]

Maybe we can rename onAfterSequenceBeforeTest to onBeforeTestFinished and onAfterSequenceAfterTest to onAfterTestFinished

Sry for bringing up a MR in draft state. I think it is now ready for review. Some things have changed after our internal review. Could you please re-check the methods on the interface, having in mind the sketched use case in #1218.

From that please give your final opinion on the feature and method names.
Thx.

[tschlat]

Looks good to me.
Well done.

[tschlat]

Still I would like a comment from @christophd as this significantly touches the heart of the framework.
Please check again for suggestions on methods and namings.

[bbortt]

well done. no change requested from my side, maybe suggested 😉 up to you.

[christophd]

okay folks, I have had a tough week last week. I am back on having a look this week. Sorry for the delay

[christophd]

Thanks a lot for the enhancement. Looks good.

As mentioned in one of the comments I would prefer to move the onTestFinish() method after the final actions have finished. It feels more natural IMO as the final actions belong to the test. Sort of a breaking change, but someone can use the new onFinalActionsStart().

Also, still not a fan of the method names onBeforeSequenceBeforeTest(). I'd suggest to use onBeforeTestStart() instead.

So we end up with this:

• onBeforeSequenceBeforeTest -> onBeforeTestStart
• onAfterSequenceBeforeTest -> onBeforeTestFinish
• onBeforeSequenceAfterTest -> onAfterTestStart
• onAfterSequenceAfterTest -> on AfterTestFinish

We could also use *End instead of *Finish. No strong opinion on that detail

Many thanks. Please let me know what you think

[tschlat]

Thanks a lot for the enhancement. Looks good.

As mentioned in one of the comments I would prefer to move the onTestFinish() method after the final actions have finished. It feels more natural IMO as the final actions belong to the test. Sort of a breaking change, but someone can use the new onFinalActionsStart().

Also, still not a fan of the method names onBeforeSequenceBeforeTest(). I'd suggest to use onBeforeTestStart() instead.

So we end up with this:

• onBeforeSequenceBeforeTest -> onBeforeTestStart
• onAfterSequenceBeforeTest -> onBeforeTestFinish
• onBeforeSequenceAfterTest -> onAfterTestStart
• onAfterSequenceAfterTest -> on AfterTestFinish

We could also use *End instead of *Finish. No strong opinion on that detail

Many thanks. Please let me know what you think

Thanks for the valuable feedback. I think we mixed implementation name 'SequenceBefore' with the interface name 'BeforeTest'. Finally, we come up with the following naming:

onTestStart (very beginning)
   onBeforeTestStart
   onBeforeTestEnd
   onTestExecutionStart
      onFinalActionStart
      onFinalActionEnd
   onTestExecutionEnd (previously onTestFinish which is now deprecated)
   onAfterTestStart
   onAfterTestEnd
onTestEnd (very end)

[tschlat]

Looks good to me

[tschlat]

Please check that the pipeline executes.

[tschlat]

Ok everyone, the pipeline is green. I'll proceed with the merge if there are no further objections.